Ben - would also be nice to allow server plugins to add to the My Apps menu on the server and player (as today they can add to radio but this does not make sense for all items).  Is this possible to consider too?

hi Adrian-

If you have a minute, I'm wondering if you can sanity check the proposed approach below I had in an earlier conversation today with Andy. (FYI, Andy is heavily working on other code right now so it will probably be Michael that ends up helping me with the SN menu code)

------
Ben K.    Andy, I'm looking at moving Applet Installer to App Gallery->3rd Party Apps. In order for this to work simply, what's going to need to happen is that an item gets sent from the server for the bottom of the App Gallery menu list. That item has a callback for a localService
1:00 PM
Ben K.    this is the patch Triode wrote a while back
View paste
+
+                       -- call local service which has previously been registered with applet manager
+                       -- this allows SlimBrowser menus to link to local applets
+                       if item.actions and item.actions.go and item.actions.go.localservice then
+                               log:debug("_actionHandler calling ", item.actions.go.localservice)
+                               menuItem:playSound("WINDOWSHOW")
+                               appletManager:callService(item.actions.go.localservice, { text = item.text })
+                               return EVENT_CONSUME
+                       end
+

the issue I have with this is backwards compatibility
if the server is delivering a 3rd Party Apps item in the App Gallery menu, and the client is running <7.6 firmware, it'll fail
One idea I had was for the item to include a callback command back to the server as well as the localservice param. It would be a fallback then-- if the client knows what to do with the localservice param, it executes that, if it's running older firmware it sends the command back to the server, where (in this case) the server sends back a window with a message that says "Update to 7.6 or greater firmware to use this feature"

What do you think of that approach?

cheers,
#!/ben

Michael, when you return from holiday I'd like to ask your assistance with the SN side of this. It shouldn't be hard and I can provide a detailed set of instructions as to what's going to be needed for this to work.

What's the purpose of the localservice pattern ? 

Will it cause problems for the third party clients like the third party iPhone and Android apps or even the Logitech iPhone/Android apps ?

Or is the intention with localservice ONLY to make it possible to add stuff to the MyApps menu ?

I'm just mentioning it so you don't forget about other non SqueezePlay clients.

I guess it will work as long as the available services are documented somewhere and they can be implemented in a non lua client. If the purpose only is to handle third party stuff which consists of a plugin part and an applet part we shouldn't have a problem, but I'm guessing the intention is to solve some other problem.

> What's the purpose of the localservice pattern ? 

Without it, it would be impossible to put a call to Applet Installer inside the App Gallery menu. The App Gallery menu is entirely generated server-side, and without this change we'd have to completely restructure the way the data comes from the server for this menu (i.e., make it a SlimMenu menu of individual items rather than a chunked 100% server-side SlimBrowse menu).

>Will it cause problems for the third party clients like the third party iPhone
and Android apps or even the Logitech iPhone/Android apps ?

It's an excellent point. We are going to need to figure out how to deal with this, as we can't display an item at all for this on non-SqueezePlay devices. AND it needs to be backwards compatible. *This point might actually be a deal breaker*, or at least I haven't figured out how to solve it sanely yet.

>Or is the intention with localservice ONLY to make it possible to add stuff to
the MyApps menu ?

No, the intention is to give more prominence to 3rd party applets, which are currently buried in Settings->Advanced->Applet Installer. Particularly this is for Triode's enhanced BBC applet, which I've not been able to get the necessary marketing traction internally (or externally with BBC) to push any "official" thing. In light of that, I'm trying to put the 3rd party applet installer in a place where more general users might see and use it.

>I'm just mentioning it so you don't forget about other non SqueezePlay clients.

I appreciate that you mentioned it, because I had forgotten about them :(

>I guess it will work as long as the available services are documented somewhere
and they can be implemented in a non lua client. If the purpose only is to
handle third party stuff which consists of a plugin part and an applet part we
shouldn't have a problem, but I'm guessing the intention is to solve some other
problem.

Nooooooo. We're not talking about plugins, but applets. This last statement is a non-starter and we should not go down this path.

to elaborate on the comment about non-squeezeplay devices and the intended fix for this bug, let me sketch out a rough list of requirements:

1. In 7.6 SP working with 7.6 SBS/SN, present an item in App Gallery called 3rd Party Apps that calls the localservice to push the Applet Installer menu.

2. In <7.5 SP working with 7.6 SBS/SN, present an item in App Gallery called 3rd Party Apps that pushes a window to screen telling user to update to 7.6 firmware to use this feature, or go to Settings->Advanced->Applet Installer.

3. In 7.6 SP working with <7.5 SBS/SN, no change in current behavior. Applet Installer is at Settings->Advanced->Applet Installer.

4. In Non Squeezeplay devices that rely on SP menus to drive their content (e.g. Logitech Squeezebox controller app for Android/iOS), present NOTHING AT ALL for this item under App Gallery.

Point 1 is pretty simple and Triode's already provided a fairly trivial patch to achieve this, pending some server-side changes.

Point 2 should be not too hard either. Newer SP should trap and execute localservice params, while older SP would fallthrough to whatever text is present to deliver after the cli request is made for the item.

Point 3 requires no work

Point 4 is the trickiest by far and I don't have a solution for this. Perhaps we'd need a hook on the server side that checks to see if the client is a real squeezeplay device, and alter both the items in App Gallery and the count as a result?

Can't we make the fallback universal so point 4 works like point 2 - I'm assuming this is what would happen by default.  So we just need the text to say something about installing apps via the menu on the device.  Otherwise as you say we need client specific responses from SN which sounds painful.

Yes, that's fine to merge points 2 and 4. But my preference is to do what point 4 is doing (not showing the item at all).

In summary, what needs to happen SN side is that the return list for App Gallery should include an item for 3rd Party Apps only if the client is a 7.6.0 or greater squeezeplay device. To have the two behaviors also requires that the count is correct for either response. I'm not aware on the server side how possible it is to identify the client down to that level of granularity-- Michael, assigning this to you for comment.

Let me add a few considerations on point 4:
1. I support dropping the item entirely, sending a message like "please install locally" could be confusing if it's not available plus Apple might find it offending since on iOS it is not ALLOWED to download and install executable code in most cases.

2. Could probably the user agent used for the communication be used here? I don't know about others but iPeng is being nice and announces itself as iPeng.

If that's not an option: iPeng identifies itself using an "userInterfaceIdiom:iPeng" tag, Adrian uses this. Other UIs could do the same. This would not be backwards compatible, though, of course...

I did some digging in SN/SBS code. The issue I'm seeing is that when a device is hooked up to SBS, there's little information about it on the SN side, because SBS is requesting the menu, not the device.

While SN receives loads of information about the player, there's nothing about the controlling device - and that's what's of interest here.

All we have is whether the player is controlled by some other device or not (x-controlled-by header - set even when fab4 is controlling itself). And this isn't very reliable, as can be seen from the comment in SBS:

	# XXX: this could be more specific, i.e. iPeng
	$client->controlledBy('squeezeplay');

x-controlled-by is always 'squeezeplay' or nothing.

So what I'd suggest is we add a x-controller-deviceinfo header similar to the x-player-deviceinfo value. It comes with a deviceid:firmware tuple. Based on this SN could decide whether a device supports the 3rd Party Apps menu or not.

Still have to figure out how everything works when connected to mysb.com directly. But I'd assume this will be easier, as we then should have more information about the controller anyway.

Could you just pass the user agent of the controller to SN via the extra header.  For SN direct and SN proxied via SBS you will then have the same information.  For SqueezePlay the user agent has the device and version number.

Created attachment 7251 [details]
Send controller's user-agent from SBS to mysb.com

Agreed, that's all the information we need ("SqueezePlay-fab4/7.6.0-r9430 (armv6l)", "iPeng/1.3", etc.).

Unfortunately can't test with mysb.com only right now. My local instance doesn't want to work any more...

 == Auto-comment from SVN commit #10515 to the network repo by mherger ==
 == http://svn.slimdevices.com/network?view=revision&revision=10515 ==

Bug: 17155
Description: add experimental support for 3rd party applet installer in app gallery

Created attachment 7253 [details]
Send controller's user-agent from SBS to mysb.com, add support for localservice menu type

same patch as before, but with additional handler for the localservice menu type. This will create a menu item like this:

            {
              actions => { go => { localservice => "appletInstallerMenu" } },
              text => "3rd Party Apps",
            },

The rest needs to be installed in SP. This item should only show up for SP 7.6 and later, no 3rd party apps or our controller app.

How about allowing the 3rd party app to define a positive list of user agents it supports.
That way you could e.g. add stuff like installing SqueezeSlave through the Web UI back.

Or you could have Apps for other platforms that allow it (that is: everything except for iOS; and then I believe there is even a loophole for iOS).

This is only for squeezeplay as it is involking an applet within squeezeplay.  The apps are applets installed within a squeezeplay client.  Hence I think only the squeezeplay user agent is relavent.

I fully understand what it is about.
But since this is for 3rd party plugins/applets, how do you know now what can be relevant? Maybe some 3rd party App will support installing stuff from a 3rd party plugin?

I, for one, could imagine a few use cases.

Isn't Ben sending the 'My Apps' menu along as part of the initial home menu.
Why not do the same for the Apps menu, then SqueezePlay 7.6 would just change the parent-node name for the Applet-Installer menu?

Am I supposed to change the user-agent now (I guess due to backwards compatibility I always sent 'SqueezePlay' in my Comet-Headers in SqueezePad) ?

> Am I supposed to change the user-agent now (I guess due to backwards
> compatibility I always sent 'SqueezePlay' in my Comet-Headers in SqueezePad) ?

Tested with SqueezePad today, and it reported as 

"x-controller-ua"     => "SqueezePad/1.3.0",

Michael: I have a patch for the MySB side of controllerUA, I'll wait until you apply your patch to 7.5. (Cannot put this in 7.6 because MySB uses 7.5.)

 == Auto-comment from SVN commit #32385 to the slim repo by mherger ==
 == http://svn.slimdevices.com/slim?view=revision&revision=32385 ==

Bug: 17155
Description: add SBS support for localservice menu item
- render localservice item for SP
- pass controller's user-agent to mysb to enable filtering of that new flag to firmwares supporting it
- backport language override in json/rpc handler from 7.6

Checked in the MySB code on test.

Ben - can you verify whether this is delivering what you expected?

I'm debugging a different issue that requires me to be on prod mysb right now. As soon as that's cleared this is next on my list.

 == Auto-comment from SVN commit #9439 to the jive repo by bklaas ==
 == http://svn.slimdevices.com/jive?view=revision&revision=9439 ==

Bug: 17155
Description: support for localservice handler on the go action to allow client-side service calls driven from the server-side

Checks out perfectly, Michael. Client side code is checked in. 

Confirmed that it does not affect the app gallery menu driven to non-squeezeplay devices, but does provides the 3rd party apps item at the bottom of App Gallery for 7.6 Squeezeplay devices.

Left to do:

We still need to push an iconStyle for this item from the server side. We're not going to get any new asset for this, so I'd recommend the settings or advanced settings icon.
 iconStyle => 'hm_settings'
  or
 iconStyle => 'hm_advancedSettings'

Also, I was considering changing the help text 
from:
        EN      3rd party applets are unsupported and should be installed at your own risk.  Please check you trust the author of an applet before installing it.

to:
        EN      3rd party apps are not supported or endorsed by Logitech, install at your own risk. Please note that a 3rd Party app installation will include and automatic restart for the installation to complete.

But then we'll need translations. Michael, can we still expect this to happen for the 7.6 release?

You might also give a hint, that 3rd party Apps won't appear in the MyApps section but provide their own individual menus.
They also need to be installed per device.

 (opposed to the other Apps, that are acivated for all players by default).

Otherwise your will confuse users, that so far (I guess most of them) are used to how App installs work.

Ben - we'll need a translation pass anyway. There are more un-translated strings around. Good point, btw....

 == Auto-comment from SVN commit #9441 to the jive repo by bklaas ==
 == http://svn.slimdevices.com/jive?view=revision&revision=9441 ==

Bug: 17155
Descirption: EN string change for help text. Needs translations from the SLT team.

The server patch seems to be in 7.6 trunk but not onebrowser?

Otherwise looking good.

Change needs to be pulled to onebrowser. Neither Andy nor Michael are available right now, and I don't want to do that pull until I've discussed it with one or both of them. My guess is that the pull to onebrowser should be straight forward though. (fyi: my checkin for bug 17199 also needs pulling to onebrowser)

Adrian, I'm looking at an email you sent me in mid-April and giving this point some consideration:

>For applets, do we now have a way to put applets and plugins into the Apps menu?  I’ve not tried this, but it would make the solution more complete.

In 7.6, the My Apps item in the home menu becomes a node with SlimMenus, which means the menu items can be a mix of server-side and client-side items.

For server-side plugins, in 7.6 you should be able to add to this menu by just giving the menu item for the plugin a node of 'myApps'. Note that this would have backwards compatibility issues though.

For client-side applets, in 7.6 you'd want the applet to add its item to the 'myApps' node, but in < 7.6 to a different location. Looking through the Squeezeplay code, I'm surprised to see that querying for firmware version hasn't been exposed as a simple System object method. So, it's not dead trivial to get the squeezeplay firmware version, but it's also not hard. Here's an example of how to query if you're running a version previous to 7.6:

=== NowPlayingApplet.lua
==================================================================
--- NowPlayingApplet.lua        (revision 50261)
+++ NowPlayingApplet.lua        (local)
@@ -41,6 +41,8 @@
 local jiveMain               = jiveMain
 local jnt                    = jnt
 
+local JIVE_VERSION     = jive.JIVE_VERSION
+
 module(..., Framework.constants)
 oo.class(_M, Applet)
 
@@ -1701,9 +1703,26 @@
 
        -- install some listeners to the window
        self:_installListeners(window)
+
+        local isBefore76 = self:_firmwareVersionChecker('^7\.[012345]')
+
+       if isBefore76 then
+               log:info('You are running revision previous to 7.6: ', JIVE_VERSION)
+       else
+               log:info('You are not running a revision previous to 7.6: ', JIVE_VERSION)
+       end
+
+
        return window
 end
 
+-- checks for version pattern in JIVE_VERSION
+function _firmwareVersionChecker(self, pattern)
+
+        local match = string.match(JIVE_VERSION, pattern)
+        return match
+end
+
 -- wrapper method to allow showNowPlaying to remain as named so the "screensaver" 
 -- can be found by the Screensaver applet correctly,
 -- while allowing the method to be called via the service API

>Looking through the
>Squeezeplay code, I'm surprised to see that querying for firmware version
>hasn't been exposed as a simple System object method. So, it's not dead trivial
>to get the squeezeplay firmware version, but it's also not hard.

heh, meant to remove those two sentences before posting last comment to this bug. It _is_ actually dead trivial to get the version by retrieving the jive.JIVE_VERSION constant and doing a pattern match against it.

Ben, I was actually looking at this yesterday... I found that there's an additional complexity as the myApps node may not be created at the time an applet's meta is run.  So I came up with:

local menus = appletManager:getAppletInstance("SlimMenus")
if menus._addMyAppsNode then
	if not menus.myAppsNode then
		menus:_addMyAppsNode()
	end
	jiveMain:addItemToNode(self.menu, 'myApps')
else
	jiveMain:addItemToNode(self.menu, 'radios')
end

This works for me and will be in the next version of BBCRadio.

Server plugins will need a server patch - if you agree with the concept I will propose a server patch.

this bit worries me:

local menus = appletManager:getAppletInstance("SlimMenus")
if menus._addMyAppsNode then

first, it violates encapsulation by drilling into an applet instance like that, but I'd also take care not to drill into an object without knowing the object is there. So, instead of

if menus._addMyAppsNode then

do

if menus and menus._addMyAppsNode then

I think the existence of myApps can be done by querying the jive.ui.HomeMenu class though. Will look into that and respond back later today on this bug.

Adrian, looking through HomeMenu.lua, I see that there is a method called getNodeItemById()

function getNodeItemById(self, id, node)
        return self.nodeTable and self.nodeTable[node] and self.nodeTable[node].items and self.nodeTable[node].items[id]

end

you can call HomeMenu methods directly through the JiveMain object, so if you wanted to check for the existence of myApps node in the home menu, you should be able to execute:

local myAppsItem = jiveMain:getNodeItemById('myApps', 'home')

can you give that a shot as an alternative to what you are doing in the BBC applet for checking on the myApps item existence?

Ben - is there anything left I should be doing?

The one remaining thing I would like to be able to do is for a SBS server plugin to be able to put itself in the myapps menu.  To do this needs server patches as currently its driven solely from the mysb.com definition of which app is enabled on each player.  Do you agree this is a sensible thing to add? - if so I will look at how it can be done..

Adrian, that feels out of scope to this bug, which I thought was solely focused on making community applets more visible, but I also don't particularly object to it. Michael, what's your opinion?

As for your previous question Michael, I think our list is down to
* making sure the server-side changes are in onebrowser and work as intended
* getting the SLT translations for text changes
* additional testing (?)

I feel comfortable that we've qualified this with 7.6/trunk & test.mysb, but the release will be onebrowser + production mysb, so not exactly a perfect QA yet...

Created attachment 7297 [details]
SBS patch to allow plugins in My Apps

Adrian - I slightly modified your patch to make sure we don't run that code on mysb. Probably not really needed, but wanted to make sure.

The change imho looks good, though I haven't really tested it.

Just for clarification:

Is "My Apps" now supposed to be a node, not a menu and am I supposed to process this node instead of using the "My Apps" menu item to open the "My Apps" menu?
And will this work in older versions of SBS, too? I realize that the menu items for the Apps always have been around or was that just for the App Gallery?

In 7.6 My Apps becomes a node. This is a change from prior versions of SBS, where My Apps was an item with a server-side cli callback.

FWIW, it's been this way for about 8 months. That change had nothing to do with this bug.

I noticed after sending Michael the patch that iPeng and SqueezePad didn't work with my patch - I believe this is because they fetch the MyApps menu rather than using the nodes defined (as SP does)?? 

Let me see if I can add enough information to the patch to make this work as is with iPeng and Squeezeplay as is - if not then they will need updating I'm afraid.

>> I believe this is because they fetch the MyApps menu rather than using the nodes defined (as SP does)?? 
SqueezePad uses the home menu nodes.
No extra logic involved for the Apps menus.
So if something doesn't show up, these nodes look different compared to all others.

I have it working on iPeng and SqueezePad by adding a menu to the xmlbrowser feed for myapps (so looks to me both of you use this...)

How is SqueezePlay handling this, when the 'My Apps' menu node was missing before?
Did it add a custom node?

Could it be that the node isn't being sent but that SqueezePlay implicitly creates the node?
This is about the only way I can imagine how this could not work in iPeng.

Wait: There is another one: if there is a node AND a menu item (for "My Apps") I believe iPeng will use the menu instead of the node.

7.6 Squeezeplay is fully backward compatible and will handle things correctly if My Apps comes as an item (7.5 and previous) or a node (7.6). If you want to look at the code logic, it's in SlimMenusApplet.lua. Look for _addMyAppsNode()

why is SqueezePlay messing with the menu on the client side?

Either the server sends My Apps as a node with all children (7.6) or as an item that loads the children (7.5).
Wouldn't the client adopt automatically without any extra logic to the corresponding server version?

For some reason I don't like the current approach.

Please try with onebrowser r32458 - this adds the capability to the server.  You need to edit your favorite plugin to have set the menu to apps or set is_app, for example:
	$class->SUPER::initPlugin(
		feed   => \&toplevel,
		tag    => 'youtube',
		menu   => 'radios',
		is_app => 1,
		weight => 5,
	);

Note there is a limitation with the ordering of the menus which I have yet to look at.  ip3k players and ipeng/squeezeplay will always put the local apps after the ones provided by mysb.com.  SP will compare weights against all of them.  Teh web interface will sort by name...  Would be nice to have the same order by all but I am not sure how to do this (perhaps ensure 3rd party app weights are always higher than built in apps - what's the highest weight Ben?)

Adrian- I don't know what the specific server logic is for weighting the MyApps items, but mine come into squeezeplay numbered from 25 to 41. If you want to push 3rd party to the bottom, I'd use numbers > 100.

Stefan, the code to deal with My Apps has been in place in 7.6 for about 9 or 10 months. I understand your objection about how it is currently implemented, but there were reasons for doing it the way I did. It's been a long time since I wrote that code, but IIRC one reason was that the My Apps node can contain client-side items as well as server-side items, so you need to create the node prior to any server connection. That is to say, you can't add a client-side item to a node with id of myApps if it doesn't exist yet.

The critical question, however, is whether you are having any current issues with 7.6 SBS and the delivery of the My Apps menu to your UI? And if so, there must be a different bug already open for this?

To be clear, this bug contains *no change* to how the My Apps menu is delivered in 7.6, only to the ability for 3rd party applets to add themselves to the node, and for the Applet installer to be housed under the App Gallery menu.

My Apps is showing fine right now with 7.6.

I was confused about Adrian writing in comment #44 that he had to add a 'My Apps' node, to make his 'plugin-Apps' appear in our clients.

to be clear I added a myapps menu to make these items appear on your clients.

Ben - what's the intended sort apps order on SP - it appears to sort by name and not weight to me?

As long is this myapps menu does not replace Bens myapps menu, I'm fine with not understanding why this is needed ;)

> Ben - what's the intended sort apps order on SP - it appears to sort by name
> and not weight to me?

This SP list is not alpha sorted, they are weight then alpha sorted. What I see in my own list is that it ends up being mostly (but not perfectly) alpha sorted, even though discrete and unique weights are being sent for each item (why, I don't know).

On SP, HomeMenu menus (of which this is one) are sorted by weight, then subsorted alpha within a given weight. 
An item with no weight is given a default weight of 100.

Ben, in Slim::Control::Jive::appsMenus I see:

my $weight = 25;
my @sorted =
 	map { $_->{weight} = $weight++; $_ } 
	sort { $a->{text} cmp $b->{text} }
	@{$menu};

Which makes me think the this is sorting by alpha and then adding weights based on this?

Assuming this is the desire I will change my checking to alpha sort the merged list with local plugins.

Ah, good find. I was looking for that code but in the wrong place.

I have no issue with what you propose re: ordering.

Can we consider this done?

I think so - lets close.  Though needs 7.6 to be released before benfit is realised :)

> I think so - lets close.  Though needs 7.6 to be released before benfit is
> realised :)

Working on it as we type :-)

please note that mysb.com needs to be running 7.6 for this to work. I sincerely hope the powers-that-be didn't play the "that hasn't been approved by QA" card on that issue.

MySB is switching to 7.6 within the next few hours. :)