Bugzilla – Bug 4112
Plugin architecture needs to be gutted.
Last modified: 2008-12-18 11:12:53 UTC
Ugh. I hate that Slim::Utils::PluginManager needs to call functions in Slim::Buttons Kevin D. legacy. it used to be a button module Dan S. Yes, I know. Which was even more all kinds of wrong. Kevin D. Slim::Buttons::Home::addSubMenu, etc shoud be a Utils Slim::Buttons::Common addMode shoudl also be a utils Dan S. No, the plugin should add itself. If the plugin needs to be on a player menu, it should do so. Kevin D. oh I see Dan S. Not any of this once-removed magic. Kevin D. I think we've had this discussion before. Case for having it this way is something like making plugins easier to write cause the server reads and loads the basic ones Dan S. At the expense of having a horrible architecture. Kevin D. yup Dan S. I'm not saying there shouldn't be utility functions to help add the menu. But the plugin should do it. 11:10 AM Kevin D. otherwise, they can be more like the button modules and have the init() called right. just like the button modules Dan S. Also, the whole UNIVERSAL::can shit needs to go, and Plugins need to become classes (and have a Plugin base class) Kevin D. and the web modules Triode has entered the room Dan S. And so if you want to have a Plugin that has a player UI, you would inherit from Slim::Plugins::PlayerUI, which inherits from Slim::Plugins::Base Kevin D. with you on all that. trick becomes how to do it without ripping everythign apart, with the limited resources and not break a nightly and have to listen to users whine about instability Dan S. Oh, this would be a "We're breaking this in the name of improving the architecture and future flexibility." Kevin D. sure, I get that Dan S. I would also want to change the Plugins namespace, so old plugins would not even try to be loaded. Everyone will need to move to the new world. Kevin D. but it's big and would require offlining a lot Dan S. No if ands or buts. And rip out the outdated and horrid wiki plugin examples Kevin D. yeah, that's the same thinking I've had for button classes create Class::Input
Additionally, the plugin loader doesn't care if the plugin has the enabled() function or not. So older plugins without sub enabled {} will be loaded.
We need to provide better feedback to the user about which plugins have been loaded and not loaded. Additionally, we need better packaging and auto-download of plugins, so people don't need to add them manually.
At the request of Dan, here's some suggestions I posted to the plugins forum (see http://forums.slimdevices.com/showthread.php?t=27389). Plugins often break when SlimServer is upgraded - whether that's a major upgrade of the server or not (although it's more common on major upgrades, obviously). The failure mode is often difficult to tie back to a plugin, as it might be seemingly unrelated things that crash in SlimServer rather than SlimServer simply refusing to start. The log is difficult for end-users to read, and plugins that fail to load are mysteriously not listed on the plugins page with no explanation. Those failures are difficult to spot and diagnose and result in wasted SD developer time identifying that the fault is a plugin - this is unhelpful to SD, plugin developers and end-users. Obviously it would be a lot of work to create a more stable interface for plugins (and may not be practical anyway given some of the deep things plugins get up to), so instead I wonder if SlimServer could be more resiliant to this kind of problem? Some ideas: * Plugins declare which server versions they work with (much like Firefox plugins). It's something of a pain for plugin authors to keep up to date with server changes, but it would avoid this problem. An enhancement to the plugins management might allow users to override what plugins have declared, but that's not a requirement here. * If SlimServer could detect the location of errors when they're thrown, could it perhaps point the finger at a plugin and suggest the plugin be checked for compatibility? (in the log if it causes SlimServer to die, or in the web interface if it seems 'minor')? * Often a plugin will throw errors during the initialise stage if the server is the wrong version - similar to the above perhaps SlimServer could signpost this more clearly somewhere. The first solution seems the most obvious to me - perhaps a new plugin method that returns a list of server versions that the plugin is comfortable with (['6.5b1', '6.5b2', '6.3.1']). Perhaps it could also contain wildcards, but that would probably defeat the point. Finally, if the method isn't there the server could behave as now and just assume it's compatible. Of course, even with the first idea the "use" of the plugin module could fail if it tries to "use" other things that fail, so some combination of the first point and last point would probably be required. For extra points, such "disabled" plugins could be identified on the slimserver plugins page of the web interface (together with the versions they want). If plugins also provided an interface that had a URL of a homepage, that page could also direct users where to look for help or a new version.
We should look into using PAR or perhaps just a bundled zip file with a Makefile.PL for all plugins.
Another thing that would be good is if slimserver would be able to handle crashing plugins a little better. Today a crash in a plugin will often bring down the whole slimserver. Of course we could require that all plugin developers make sure to add eval{} around their code and don't write plugins that crashes, but I feel that it would be better if this could be handled in the slimserver whenever it calls on of the plugin callbacks. However to make this work there need to be some sort of crash log available where the plugin developer could get information about the crash so the problem can be corrected.
Dan : The real Plugin solution would be to also blow away that directory, but we don't have an alternate location on Non-Mac platforms currently. Dean: consider creating a new plugins folder for the plugins that come with slimserver. Dan : Like: Slim/Plugins/... ? Dean: then use the old plugins folder for user-installed plugins AND also rm the system ones that it finds there during an install.
Please also consider moving 3rd party plugins settings and data _out_ of the slimserver.pref file. Also, an automated means of uninstalling a plugin should be created either by the installation process or else required of all plugins so that they clean up any tables created in the database, any files installed in the server tree, and any settings and data stored in prefs files.
*** Bug 4414 has been marked as a duplicate of this bug. ***
*** Bug 4401 has been marked as a duplicate of this bug. ***
Hey all, Sorry I'm late to the party. I've only recently started messing around with this stuff. With the pain of updating a plugin fresh in my mind, I thought I'd add some of my thoughts on the subject. I don't necessarily think the plugin interface is all that bad. The biggest shortcomings are the lack of a documented API. Whether or not the plugin interface is exposed through a OO mechanism or as a structured API is irrelevant if a plugin author does not know which interfaces to implement and which server APIs are public. A simple way to declare a public API would be to use different module names for the two. For example, Slim::Public::Player::Client and Slim::Player::Squeezebox. A blanket statement could then be made, "All API not in 'Public' packages are private and subject to change at any time." The benefit of declaring a public API is that you now have a contract with plugin developers. You have an idea of what functionality they might be using and they have some piece of mind regarding their usage of those APIs. This "contract" can be further enhanced by declaring rules regarding the stability of the APIs from release to release. For example: No APIs will be removed between minor releases, although they may be modified in a non-breaking, backward compatible way. Deprecated APIs may be removed in the major release following the release in which they were deprecated. With all that out of the way, I think you could enhance the current API by simply providing wrappers for the existing functionality in the server. This may not pretty up the existing code base, but you know that code works (even if it's a bit clunky to use). This approach will also help with backward compatibility for existing plugins. (For example, I've created a simple wrapper class for interacting with the various built-in INPUT modes in the new xmradio plugin. It's not pretty either, but it serves to illustrate my point.) Once you've got those wrappers in place, you have the beginnings of an API contract that can be evolved and maintained from release to release, giving you free reign to modify the code beneath them. As for supporting existing plugins, I would think you could implement a LegacyPlugin class that would delegate to the methods declared in older plugin modules. An instance of this class would be created for each legacy plugin installed on the server. Assuming no other API breakage occurs, this would be a nice way to support legacy plugins in the next release. It would also give you a means to handle any errors in an appropriate manner (e.g. disabling the plugin if one of its methods generates an exception). Lastly, I would add that I think it's important to provide backward compatibility for existing plugins. I know from experience that customers will be unwilling to upgrade if their favorite plugin doesn't work with the latest version. Best regards, Rob
Settings as 'fixed' since the new plugin architecture has been implemented in the 7.0 beta. This bug will be marked 'closed' after the release of 7.0 A portion of this bug, the ability to easily install plugins, has been split off into bug 5809.
This bug is being closed since it was resolved for a version which is now released! Please download the new version of SqueezeCenter (formerly SlimServer) at http://www.slimdevices.com/su_downloads.html If you are still seeing this bug, please re-open it and we will consider it for a future release.