Hi Klaus,
I was asked, if I could expose some functionality of epgsearch (searching the EPG, calling the extended timer edit menu) to other plugins. I plan to do this, like the timeline plugin does to check for timer conflicts in other plugins:
bool cPluginTimeline::ProcessArgs(int argc, char *argv[]) { // Implement command line argument processing here if applicable. if (argc==1 && argv!=NULL && !strcmp(argv[0],"timeline_command_interface") && !strcmp(argv[1],"conflicts")) { // yes, this is an ugly hack! return hasConflicts(); } return true; }
As you see, it uses the ProcessArgs function for this. Not very nice, but I think it's currently the only way. Would it be possible, to add a new function to cPlugin, like 'Interface', that works the same way, but is intended for this special job? It should also have a variable argument list. This would even allow to return pointers to cOsdMenus. Why reinvent the wheel, if any other plugin already does the job? ;-)
BR,
Christian
cwieninger@gmx.de wrote:
I was asked, if I could expose some functionality of epgsearch (searching the EPG, calling the extended timer edit menu) to other plugins. I plan to do this, like the timeline plugin does to check for timer conflicts in other plugins:
Another way to tunnel information from one plugin to another is to use SetupParse("==epgsearch-tunnel-v1.0==",(char*)&data) for it. If, for some reason, the plugin doesn't know this tunnel (wrong version, wrong plugin), the function will most likely ignore this call and return false.
A far better way would be of course if VDR's cPlugin would have such a general purpose interface. My approach would be like this:
virtual bool ExtensionInterface(const char *ExtensionId, void *Data)
Plugins return false unless ExtensionId matches a known unique protocol ID. If the plugin supports this protocol, true will be returned in any case. If *Data is not NULL, it points to a protocol-specific buffer. If *Data is NULL, this is just a protocol-supported call.
cPluginManager should have methods to call ExtensionInterface for all plugins (broadcast-like), or for all plugins until one plugin returns true (find one that offers the interface).
@Klaus: I can implement that if interested.
Cheers,
Udo
Udo Richter wrote:
cwieninger@gmx.de wrote:
I was asked, if I could expose some functionality of epgsearch (searching the EPG, calling the extended timer edit menu) to other plugins. I plan to do this, like the timeline plugin does to check for timer conflicts in other plugins:
Another way to tunnel information from one plugin to another is to use SetupParse("==epgsearch-tunnel-v1.0==",(char*)&data) for it. If, for some reason, the plugin doesn't know this tunnel (wrong version, wrong plugin), the function will most likely ignore this call and return false.
A far better way would be of course if VDR's cPlugin would have such a general purpose interface. My approach would be like this:
virtual bool ExtensionInterface(const char *ExtensionId, void *Data)
Plugins return false unless ExtensionId matches a known unique protocol ID. If the plugin supports this protocol, true will be returned in any case. If *Data is not NULL, it points to a protocol-specific buffer. If *Data is NULL, this is just a protocol-supported call.
cPluginManager should have methods to call ExtensionInterface for all plugins (broadcast-like), or for all plugins until one plugin returns true (find one that offers the interface).
@Klaus: I can implement that if interested.
Why don't you just give your plugin a dedicated function that does exactly what you want? You'll just need to typecast the cPlugin pointer you have to the particular type and can then call that plugin's additional functions.
Or am I missing something here?
Klaus
Hi,
Why don't you just give your plugin a dedicated function that does exactly what you want? You'll just need to typecast the cPlugin pointer you have to the particular type and can then call that plugin's additional functions.
Or am I missing something here?
Klaus
but this would force the 'client'-plugin, that wants to use some functions of the 'server'-plugin, to include a header, that defines the extended plugin-class. Or how should the client-plugin know about the additional functions? And what, if the server-plugin isn't installed?
I agree to Udo, that a general purpose interface would be a better solution.
BR,
Christian
Christian Wieninger wrote:
Hi,
Why don't you just give your plugin a dedicated function that does exactly what you want? You'll just need to typecast the cPlugin pointer you have to the particular type and can then call that plugin's additional functions.
Or am I missing something here?
Klaus
but this would force the 'client'-plugin, that wants to use some functions of the 'server'-plugin, to include a header, that defines the extended plugin-class. Or how should the client-plugin know about the additional functions?
It would have to know about exactly which parameters to provide, anyway. Where's the problem with including the proper header file?
And what, if the server-plugin isn't installed?
Then you won't get it through cPluginManager::GetPlugin(const char *Name). Or how do you get the plugin, anyway?
I agree to Udo, that a general purpose interface would be a better solution.
I don't - at least not yet. Including the proper header file and calling a dedicated function seems much more reasonable to me.
Klaus
Hi,
It would have to know about exactly which parameters to provide, anyway. Where's the problem with including the proper header file?
I agree with you, if the functions of the server-plugin are an essential part of the client plugin. But :-) Take timeline for example and its timer conflict check. If its installed, then epgsearch checks periodically if there are any timer conflicts. If not, no problem. There's no dependence at compile- or runtime. Searching for repeats of using the extended timer edit menu is the same thing. If epgsearch is present, a plugin can use this functions.
BR,
Christian
Christian Wieninger wrote:
Hi,
It would have to know about exactly which parameters to provide, anyway. Where's the problem with including the proper header file?
I agree with you, if the functions of the server-plugin are an essential part of the client plugin. But :-) Take timeline for example and its timer conflict check. If its installed, then epgsearch checks periodically if there are any timer conflicts. If not, no problem. There's no dependence at compile- or runtime. Searching for repeats of using the extended timer edit menu is the same thing. If epgsearch is present, a plugin can use this functions.
But it has to know the semantics of that plugin in any case. You can't just put some random data into some abstract interface and expect something particular to happen ;-) So why not have the plugin that provides a certain functionality that's of interest to other plugins advertise this like any other library would do?
The clean method is for the "server" to provide a proper public member function and for the "client" to include the plugin's header file. Determining whether or not a plugin is present doesn't change through that - you already do that now, don't you?
Klaus
Klaus Schmidinger wrote:
Why don't you just give your plugin a dedicated function that does exactly what you want? You'll just need to typecast the cPlugin pointer you have to the particular type and can then call that plugin's additional functions.
How to check that the plugin is really what it seems to be?
With these multiple-symbol-contexts, I'm not sure if the safe way to dynamic_cast<cSomePlugin>(Plugin) would work, so you have to blindly cast to cSomePlugin. And this class is declared twice: once in the server plugin, once in the client plugin. If these two declarations don't match exactly, the result is unpredictable and will most likely crash. Even a small update of the server plugin that is not reflected to the client plugin can cause chaos.
Cheers,
Udo
Klaus Schmidinger schrieb:
Why don't you just give your plugin a dedicated function that does exactly what you want? You'll just need to typecast the cPlugin pointer you have to the particular type and can then call that plugin's additional functions.
Or am I missing something here?
I tried to do the communication like this for softplay and softdevice.
The problem here is that the plugins don't have acces to the symbols of each other. So plugin1 cannot call plugin2->Somefunction(), except this plugin is dlopened with the flag RTLD_GLOBAL to make the symbols available to everyone. But this is not recommended... The other possibility is to actually link one plugin to the other, which I also don't like.
I would appreciate very much a extension of cPlugin like Udo suggested. I think it would be the best solution also for softplay/softdevice.
Martin
----- Original Message ----- From: "Klaus Schmidinger" Klaus.Schmidinger@cadsoft.de To: vdr@linuxtv.org Sent: Thursday, August 18, 2005 4:42 PM Subject: Re: [vdr] Feature request: suggestion for cPlugin
The clean method is for the "server" to provide a proper public member function and for the "client" to include the plugin's header file. Determining whether or not a plugin is present doesn't change through that - you already do that now, don't you?
but the big disadvantage are the dependencies at compile time. If the user wants interaction between plugins he would have to set any defines in the makefile. Thats not the case if there is a general purpose interface.
You may be right concerning the semantics, but why explaining the semantics only in a header file? If there's a general interface, the documentation of the protocol could also be part of the readme (or a readme for developers).
BR,
Christian
Klaus Schmidinger wrote:
You can't just put some random data into some abstract interface and expect something particular to happen ;-)
Thats why some unique identification string is necessary, to make sure both plugins talk about the same struct for communication.
The clean method is for the "server" to provide a proper public member function and for the "client" to include the plugin's header file.
How? #include "../epgsearch/plugin.h"? First, the symlink name "epgsearch" is user-dependant, any may be set differently based on personal taste. Second, now the plugin depends on epgsearch and cannot run without it.
Just keeping a local copy of plugin.h? This leads to dangerous versioning troubles, see other post.
Determining whether or not a plugin is present doesn't change through that - you already do that now, don't you?
Another point: With a dedicated interface, plugins can offer general interfaces like "timer-conflicts" or "search-repeats", so any plugin can use this feature and any plugin can provide this feature.
Cheers,
Udo
Udo Richter wrote:
cwieninger@gmx.de wrote:
I was asked, if I could expose some functionality of epgsearch (searching the EPG, calling the extended timer edit menu) to other plugins. I plan to do this, like the timeline plugin does to check for timer conflicts in other plugins:
Another way to tunnel information from one plugin to another is to use SetupParse("==epgsearch-tunnel-v1.0==",(char*)&data) for it. If, for some reason, the plugin doesn't know this tunnel (wrong version, wrong plugin), the function will most likely ignore this call and return false.
A far better way would be of course if VDR's cPlugin would have such a general purpose interface. My approach would be like this:
virtual bool ExtensionInterface(const char *ExtensionId, void *Data)
Plugins return false unless ExtensionId matches a known unique protocol ID. If the plugin supports this protocol, true will be returned in any case. If *Data is not NULL, it points to a protocol-specific buffer. If *Data is NULL, this is just a protocol-supported call.
cPluginManager should have methods to call ExtensionInterface for all plugins (broadcast-like), or for all plugins until one plugin returns true (find one that offers the interface).
@Klaus: I can implement that if interested.
Well, since I'm not going to use that interface, anyway, and it shouldn't have any impact on plain vanilla VDR, just go ahead and send a patch.
And don't forget to adjust PLUGINS.html ;-)
Klaus
Klaus Schmidinger wrote:
Udo Richter wrote:
A far better way would be of course if VDR's cPlugin would have such a general purpose interface. My approach would be like this:
virtual bool ExtensionInterface(const char *ExtensionId, void *Data)
Plugins return false unless ExtensionId matches a known unique protocol ID. If the plugin supports this protocol, true will be returned in any case. If *Data is not NULL, it points to a protocol-specific buffer. If *Data is NULL, this is just a protocol-supported call.
cPluginManager should have methods to call ExtensionInterface for all plugins (broadcast-like), or for all plugins until one plugin returns true (find one that offers the interface).
@Klaus: I can implement that if interested.
Well, since I'm not going to use that interface, anyway, and it shouldn't have any impact on plain vanilla VDR, just go ahead and send a patch.
And don't forget to adjust PLUGINS.html ;-)
Ok, here's a first implementation of what I had in mind. The attached patch implements the interface for VDR 1.3.20 and newer. PLUGINS.html and newplugin still need to be updated. ;)
The attached vdr-extintf-0.1.0.tgz implements two sample plugins that do some communication: Direct communication to a specific plugin, detecting presence of plugins that offer a service, use a service provided by some plugin and broadcast a message to all interested plugins. Install as usual. Load the two plugins with vdr -P extcli -P extsvr.
@Plugin developers: Suggestions, reviews and comments are welcome!
Cheers,
Udo
diff -au vdr-1.3.29-orig/plugin.c vdr-1.3.29/plugin.c --- vdr-1.3.29-orig/plugin.c 2005-01-30 15:05:20.000000000 +0100 +++ vdr-1.3.29/plugin.c 2005-08-18 18:20:05.000000000 +0200 @@ -99,6 +99,11 @@ Setup.Store(Name, Value, this->Name()); }
+bool cPlugin::ExtensionInterface(const char *ExtensionId, void *Data) +{ + return false; +} + void cPlugin::RegisterI18n(const tI18nPhrase * const Phrases) { I18nRegister(Phrases, Name()); @@ -372,6 +377,43 @@ return NULL; }
+cPlugin *cPluginManager::CallFirstExtension(const char *ExtensionId, void *Data) +{ + if (pluginManager) { + for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { + cPlugin *p = dll->Plugin(); + if (p) + if (p->ExtensionInterface(ExtensionId, Data)) return p; + } + } + return NULL; +} + +cPlugin *cPluginManager::CallPluginExtension(const char *PluginName, const char *ExtensionId, void *Data) +{ + if (pluginManager) { + for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { + cPlugin *p = dll->Plugin(); + if (p && strcmp(p->Name(), PluginName) == 0) + if (p->ExtensionInterface(ExtensionId, Data)) return p; + } + } + return NULL; +} + +bool cPluginManager::CallAllExtensions(const char *ExtensionId, void *Data) +{ + bool found=false; + if (pluginManager) { + for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { + cPlugin *p = dll->Plugin(); + if (p) + if (p->ExtensionInterface(ExtensionId, Data)) found=true; + } + } + return found; +} + void cPluginManager::StopPlugins(void) { for (cDll *dll = dlls.Last(); dll; dll = dlls.Prev(dll)) { diff -au vdr-1.3.29-orig/plugin.h vdr-1.3.29/plugin.h --- vdr-1.3.29-orig/plugin.h 2005-01-30 15:03:48.000000000 +0100 +++ vdr-1.3.29/plugin.h 2005-08-18 20:22:20.000000000 +0200 @@ -50,6 +50,8 @@
void RegisterI18n(const tI18nPhrase * const Phrases);
+ virtual bool ExtensionInterface(const char *ExtensionId, void *Data = NULL); + static void SetConfigDirectory(const char *Dir); static const char *ConfigDirectory(const char *PluginName = NULL); }; @@ -88,8 +90,12 @@ static bool HasPlugins(void); static cPlugin *GetPlugin(int Index); static cPlugin *GetPlugin(const char *Name); + static cPlugin *CallFirstExtension(const char *ExtensionId, void *Data = NULL); + static cPlugin *CallPluginExtension(const char *PluginName, const char *ExtensionId, void *Data = NULL); + static bool CallAllExtensions(const char *ExtensionId, void *Data = NULL); void StopPlugins(void); void Shutdown(void); }; +#define PATCH_EXTENSIONINTERFACE
#endif //__PLUGIN_H
Udo Richter schrieb:
The attached vdr-extintf-0.1.0.tgz implements two sample plugins that do some communication: Direct communication to a specific plugin, detecting presence of plugins that offer a service, use a service provided by some plugin and broadcast a message to all interested plugins. Install as usual. Load the two plugins with vdr -P extcli -P extsvr.
@Plugin developers: Suggestions, reviews and comments are welcome!
I like interface for cPlugin::ExtensionInterface() it will be very usefull for the softplay/softdevice communication!
In my opinion the method CallPluginExtension() is not needed, the same can easily done by just calling cPluginManager::GetPlugin("name")->ExtensionInterface().
About the other methods CallFirstExtension() or CallAllExtension(), I'm not sure. Currently I don't think that I will need them. But who knows? If they are there, I think it would be nice to be able to know which plugin actually answered the request, so that following request can be sent directly to that plugin. That way one could first query for a specific extension and then send all requests directly.
Bye, Martin
Hi,
@Plugin developers: Suggestions, reviews and comments are welcome!
Great job! Thats exactly how it should be.
In my opinion the method CallPluginExtension() is not needed, the same can easily done by just calling cPluginManager::GetPlugin("name")->ExtensionInterface().
ok, but perhaps it looks better ;-)
If they are there, I think it would be nice to be able to know which plugin actually answered the request, so that following request can be sent directly to that plugin. That way one could first query for a specific extension and then send all requests directly.
I think so too. Perhaps CallAllExtensions could also return a pointer to the plugin like CallFirstExtension does.
Thanks again. The next release of epgsearch will support this patch, and I hope I can soon drop the currently needed #ifdef PATCH_EXTENSIONINTERFACE ;-)
Hope other developers also integrate it.
BR,
Christian
Martin Wache wrote:
In my opinion the method CallPluginExtension() is not needed, the same can easily done by just calling cPluginManager::GetPlugin("name")->ExtensionInterface().
The only (very minor) difference is that your call uses the first plugin of that name, while CallPluginExtension() queries all plugins of that name until one answers.
I think it would be nice to be able to know which plugin actually answered the request, so that following request can be sent directly to that plugin.
CallPluginExtension() and CallFirstExtension() actually return a pointer to the plugin that answered the request, or NULL if none answered.
If you're worried about speed, you could pass through a pointer to a callback function, so further communication uses the callback and not the interface.
Christian Wieninger wrote:
I think so too. Perhaps CallAllExtensions could also return a pointer to the plugin like CallFirstExtension does.
The point of CallAllExtensions() is that the call goes to all extensions, not just one. It would need a whole list of plugins to solve this.
Thanks again. The next release of epgsearch will support this patch, and I hope I can soon drop the currently needed #ifdef PATCH_EXTENSIONINTERFACE ;-)
I've used #if VDRVERSNUM >= 10330 || defined(PATCH_EXTENSIONINTERFACE) in the sources by purpose, so the code will be used for VDR 1.3.30 and any previous version that is patched. Why drop backwards compatibility?
Cheers,
Udo
Martin Wache wrote:
In my opinion the method CallPluginExtension() is not needed, the same can easily done by just calling cPluginManager::GetPlugin("name")->ExtensionInterface().
Why didn't I see that before? Obviously, this call also misses an important NULL test, so your way blows up to:
cPlugin *p=cPluginManager::GetPlugin("name"); if (p) p->ExtensionInterface(...);
In the end, these functions are just convenient helper functions. All these functions including GetPlugin("Name") can be substituted just by using GetPlugin(int) and a few more lines of code.
Cheers,
Udo
Udo Richter wrote:
... Ok, here's a first implementation of what I had in mind. The attached patch implements the interface for VDR 1.3.20 and newer. PLUGINS.html and newplugin still need to be updated. ;)
The attached vdr-extintf-0.1.0.tgz implements two sample plugins that do some communication: Direct communication to a specific plugin, detecting presence of plugins that offer a service, use a service provided by some plugin and broadcast a message to all interested plugins. Install as usual. Load the two plugins with vdr -P extcli -P extsvr.
@Plugin developers: Suggestions, reviews and comments are welcome!
Cheers,
Udo
...
- virtual bool ExtensionInterface(const char *ExtensionId, void *Data = NULL);
...
- static cPlugin *CallFirstExtension(const char *ExtensionId, void *Data = NULL);
- static cPlugin *CallPluginExtension(const char *PluginName, const char *ExtensionId, void *Data = NULL);
- static bool CallAllExtensions(const char *ExtensionId, void *Data = NULL);
I'm not really fond of this "extension" thing. IMHO this is more of an "inter plugin communication" thing than an "extension". Or, maybe "service" is the right name. After all, that's what it is: a plugin can offer a service that other plugins can make use of.
So I suggest s/Extension/Service/g, and plain "Service" instead of "ExtensionInterface".
You also might want to give some guidelines as to how to create those "ServiceIDs" - otherwise most likely every plugin will come up with a totally different scheme.
Klaus
Udo Richter wrote:
Martin Wache wrote:
In my opinion the method CallPluginExtension() is not needed, the same can easily done by just calling cPluginManager::GetPlugin("name")->ExtensionInterface().
The only (very minor) difference is that your call uses the first plugin of that name, while CallPluginExtension() queries all plugins of that name until one answers.
There is only _one_ plugin of any given name. So I also think that CallPluginExtension() is superfluous.
Any caller who wants to direct a service call to a particular plugin should first check whether that plugin is there, anyway. Otherwise it wouldn't know whether the request was denied because the parameters were wrong, or whether the plugin wasn't there at all.
cPlugin *Plugin = cPluginManager::GetPlugin("someplugin"); if (Plugin) Plugin->Service("someserviceid", somedata);
I think it would be nice to be able to know which plugin actually answered the request, so that following request can be sent directly to that plugin.
CallPluginExtension() and CallFirstExtension() actually return a pointer to the plugin that answered the request, or NULL if none answered.
If you're worried about speed, you could pass through a pointer to a callback function, so further communication uses the callback and not the interface.
Christian Wieninger wrote:
I think so too. Perhaps CallAllExtensions could also return a pointer to the plugin like CallFirstExtension does.
The point of CallAllExtensions() is that the call goes to all extensions, not just one. It would need a whole list of plugins to solve this.
I also think that returning a bool telling the caller whether _any_ plugin has processed the call should be enough.
Thanks again. The next release of epgsearch will support this patch, and I hope I can soon drop the currently needed #ifdef PATCH_EXTENSIONINTERFACE ;-)
I've used #if VDRVERSNUM >= 10330 || defined(PATCH_EXTENSIONINTERFACE) in the sources by purpose, so the code will be used for VDR 1.3.30 and any previous version that is patched. Why drop backwards compatibility?
Please note my suggestion regarding the names (s/Extension/Service/g) in my reply to Udo.
Klaus
Klaus Schmidinger wrote:
Udo Richter wrote:
... Ok, here's a first implementation of what I had in mind. The attached patch implements the interface for VDR 1.3.20 and newer. PLUGINS.html and newplugin still need to be updated. ;)
The attached vdr-extintf-0.1.0.tgz implements two sample plugins that do some communication: Direct communication to a specific plugin, detecting presence of plugins that offer a service, use a service provided by some plugin and broadcast a message to all interested plugins. Install as usual. Load the two plugins with vdr -P extcli -P extsvr.
@Plugin developers: Suggestions, reviews and comments are welcome!
Cheers,
Udo
...
- virtual bool ExtensionInterface(const char *ExtensionId, void *Data
= NULL); ...
- static cPlugin *CallFirstExtension(const char *ExtensionId, void
*Data = NULL);
- static cPlugin *CallPluginExtension(const char *PluginName, const
char *ExtensionId, void *Data = NULL);
- static bool CallAllExtensions(const char *ExtensionId, void *Data =
NULL);
I'm not really fond of this "extension" thing. IMHO this is more of an "inter plugin communication" thing than an "extension". Or, maybe "service" is the right name. After all, that's what it is: a plugin can offer a service that other plugins can make use of.
So I suggest s/Extension/Service/g, and plain "Service" instead of "ExtensionInterface".
Arghh - come to think of it, "service ID" is already used in a different context.
Well, maybe somebody finds a better name. I'm still not happy with "extension".
Klaus
Hi,
You also might want to give some guidelines as to how to create those "ServiceIDs" - otherwise most likely every plugin will come up with a totally different scheme.
Klaus
I also think this should be done, and atleast a version info has to be part of the interface to avoid problems with changed protocols in newer plugins. Udo made it part of the service string itself in his sample plugins. Perhaps it should be a separate value. Don't know ;-)
Since this interface can not only be used to call a service of the server plugin, but also to broadcast a message from the client to possibly many servers, this should also be considered with respect to the naming problem.
Some suggestions:
- CommunicationInterface - InterchangeInterface - ExchangeService
BR,
Christian
Klaus Schmidinger wrote:
There is only _one_ plugin of any given name. So I also think that CallPluginExtension() is superfluous.
Wrong. One plugin can be loaded multiple times. ;) Theoretically, a plugin _may_ behave differently each time it is loaded, based on an internal counter or on command line parameters.
Of course two plugins cannot share the same name without dirty tricks.
Cheers,
Udo
"Christian Wieninger" cwieninger@gmx.de wrote:
- CommunicationInterface
- InterchangeInterface
- ExchangeService
bool cPlugin::IPC(const char *MsgId, void *Data)
b.t.w. IPC stands for inter plugin communication :o)
Udo Richter wrote:
Klaus Schmidinger wrote:
There is only _one_ plugin of any given name. So I also think that CallPluginExtension() is superfluous.
Wrong. One plugin can be loaded multiple times. ;) Theoretically, a plugin _may_ behave differently each time it is loaded, based on an internal counter or on command line parameters.
Of course two plugins cannot share the same name without dirty tricks.
Gee, I hate it when that happens ;-) I really thought I had that covered.
Since cPluginManager::GetPlugin(const char *Name) only makes real sense if there can't be two plugins with the same name, I guess loading the same plugin more than once isn't a good idea. It might also cause irritation with the plugin's setup parameters.
I'd therefore suggest that we simply assume that each plugin is loaded only once, and if somebody really thinks he/she has to load a plugin twice, they should know what they are doing...
Klaus
Christian Wieninger wrote:
Hi,
You also might want to give some guidelines as to how to create those "ServiceIDs" - otherwise most likely every plugin will come up with a totally different scheme.
Klaus
I also think this should be done, and atleast a version info has to be part of the interface to avoid problems with changed protocols in newer plugins. Udo made it part of the service string itself in his sample plugins. Perhaps it should be a separate value. Don't know ;-)
Well, I'll leave that to Udo ;-) My original arguments regarding the use of dedicated functions were dismissed, saying that this would require including header files for definitions. Well, if you take a look at Udo's example plugins, there is
struct ReportBoredPlugin { cPlugin *BoredPlugin; };
struct AddService { int a,b; int sum; };
in both of them. So there's a code duplication right there, and if the server decides to change something, the client will have to change it's code as well. It will be fun to watch this stuff go through protocol changes... ;-)
Since this interface can not only be used to call a service of the server plugin, but also to broadcast a message from the client to possibly many servers, this should also be considered with respect to the naming problem.
Some suggestions:
- CommunicationInterface
- InterchangeInterface
- ExchangeService
Hmm, still no "oh yes!" feeling here.
Maybe my original "service" suggestion isn't that obsolete, yet. What drove me off of it was the "ServiceID", which is already used for something completely different. But the generic word "service" should still be able to be used.
So how about this:
cPlugin:
virtual bool Service(const char *Id, void *Data = NULL);
cPluginManager:
static cPlugin *CallFirstService(const char *Id, void *Data = NULL); static cPlugin *CallPluginService(const char *PluginName, const char *Id, void *Data = NULL); static bool CallAllServices(const char *Id, void *Data = NULL);
Actually CallPluginService() should be superfluous because the caller could first get the plugin by name and then call its Service() function (thus allowing it to actually know whether there is such a plugin at all). So personally I'd vote for not implementing CallPluginService(), but if you absolutely insist, I won't argue any more.
Klaus
Klaus Schmidinger wrote:
My original arguments regarding the use of dedicated functions were dismissed, saying that this would require including header files for definitions.
As far as I can tell, Martin's argument is the k.o. criteria for this, as access to member functions requires both plugins to share a linker symbol context. Sharing just the headers imho only gives access to member variables and virtual methods derived from cPlugin. One could use a function pointer, but thats not really elegant.
Well, if you take a look at Udo's example plugins, there is struct ReportBoredPlugin struct AddService in both of them. So there's a code duplication right there, and if the server decides to change something, the client will have to change it's code as well. It will be fun to watch this stuff go through protocol changes... ;-)
A dedicated struct is a lot safer than the whole class, as it wont change that frequently. And the documentation will clearly warn, that any change to the struct requires changing the ID string too.
Hmm, still no "oh yes!" feeling here.
Maybe my original "service" suggestion isn't that obsolete, yet.
Ok, I'll come up with another idea: Why not call it a message interface?
cPluginManager::SendMessage(...) cPluginManager::BroadcastMessage(...) cPlugin::ProcessMessage(...) or cPlugin::Message(...)
So personally I'd vote for not implementing CallPluginService(), but if you absolutely insist, I won't argue any more.
Since most people think it is not needed, I agree to drop it. I've added it only for completeness anyway.
Cheers,
Udo
Udo Richter wrote:
Klaus Schmidinger wrote:
My original arguments regarding the use of dedicated functions were dismissed, saying that this would require including header files for definitions.
As far as I can tell, Martin's argument is the k.o. criteria for this, as access to member functions requires both plugins to share a linker symbol context. Sharing just the headers imho only gives access to member variables and virtual methods derived from cPlugin. One could use a function pointer, but thats not really elegant.
Well, if you take a look at Udo's example plugins, there is struct ReportBoredPlugin struct AddService in both of them. So there's a code duplication right there, and if the server decides to change something, the client will have to change it's code as well. It will be fun to watch this stuff go through protocol changes... ;-)
A dedicated struct is a lot safer than the whole class, as it wont change that frequently. And the documentation will clearly warn, that any change to the struct requires changing the ID string too.
Ok, let's end this - there shall be those functions you suggested.
Hmm, still no "oh yes!" feeling here.
Maybe my original "service" suggestion isn't that obsolete, yet.
Ok, I'll come up with another idea: Why not call it a message interface?
cPluginManager::SendMessage(...) cPluginManager::BroadcastMessage(...) cPlugin::ProcessMessage(...) or cPlugin::Message(...)
Well, "Message" IMHO implies that this is sort of a one way thing, while "Service" can go either way. In your example you have a plugin that adds numbers. Now if a client "sends a message" to that server, saying "add 2 and 3", the server might think "well, nice message". OTOH if the client "requests the service" of "add 2 and 3", this (at least to me) implies much more that there is a result to be expected.
If I'm not mistaken the whole point of this interface is to allow one plugin to make use of a service provided by another plugin. Therefore I'd still consider "service" the better choice.
Klaus
Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Ok, I'll come up with another idea: Why not call it a message interface?
cPluginManager::SendMessage(...) cPluginManager::BroadcastMessage(...) cPlugin::ProcessMessage(...) or cPlugin::Message(...)
Well, "Message" IMHO implies that this is sort of a one way thing, while "Service" can go either way. In your example you have a plugin that adds numbers. Now if a client "sends a message" to that server, saying"add 2 and 3", the server might think "well, nice message". OTOH if the client "requests the service" of "add 2 and 3", this (at least to me) implies much more that there is a result to be expected.
i once designed a RPC library that supported both semantics you describe here. there were RPC::Functions and RPC::Messages. the difference was that messages were simple one way atoms, though could be sent ither way. functions on the other hand, were strictly syncronous, and the caller was blocked, until the "service provider" sent back the return argument.
in the vdr context, this would be even simpler, as there is no network magic needed to make things work reliably. for example (keeping udos syntax):
cPluginManager::SendMessage(PluginID dest, MsgID msg, void *data) cPluginManager::BroadcastMessage(MsgID msg, void *data)
cPluginManager::CallFunction(void *ret, PluginID dest, FnID id, void *data)
i think other rpc-implementations do things similar, like D-BUS, DCOP.
so despite the fact that somebody might confuse cPlugin::Messages with OSD::Messages, i think the term "Messages" fit well into this context.
clemens
Clemens Kirchgatterer wrote:
Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Ok, I'll come up with another idea: Why not call it a message interface?
cPluginManager::SendMessage(...) cPluginManager::BroadcastMessage(...) cPlugin::ProcessMessage(...) or cPlugin::Message(...)
Well, "Message" IMHO implies that this is sort of a one way thing, while "Service" can go either way. In your example you have a plugin that adds numbers. Now if a client "sends a message" to that server, saying"add 2 and 3", the server might think "well, nice message". OTOH if the client "requests the service" of "add 2 and 3", this (at least to me) implies much more that there is a result to be expected.
i once designed a RPC library that supported both semantics you describe here. there were RPC::Functions and RPC::Messages. the difference was that messages were simple one way atoms, though could be sent ither way. functions on the other hand, were strictly syncronous, and the caller was blocked, until the "service provider" sent back the return argument.
in the vdr context, this would be even simpler, as there is no network magic needed to make things work reliably. for example (keeping udos syntax):
cPluginManager::SendMessage(PluginID dest, MsgID msg, void *data) cPluginManager::BroadcastMessage(MsgID msg, void *data)
cPluginManager::CallFunction(void *ret, PluginID dest, FnID id, void *data)
i think other rpc-implementations do things similar, like D-BUS, DCOP.
so despite the fact that somebody might confuse cPlugin::Messages with OSD::Messages, i think the term "Messages" fit well into this context.
Well, it's the same as always - if you start discussing such things as naming stuff, things get tedious... ;-)
I'll give up - name it whatever you want and make it whatever you like. I won't be needing nor using this stuff, anyway, so what do I care.
Klaus
Hi.
Ok, I'm now aiming for the service variant:
bool cPlugin::Service(const char *Id, void *Data = NULL) static cPlugin *cPluginManager::CallFirstService(const char *Id, void *Data = NULL); static bool cPluginManager::CallAllServices(const char *Id, void *Data = NULL);
(I thought about CallFirstService being stupid, but could not find anything better that makes clear, that just one plugin will handle this)
New patch including docs etc. later this day.
There's a minor taste issue that I want to bring up before its too late. The current interface suggests (but not enforces) that Service("someid",NULL) asks whether this protocol is supported or not, and doesn't cause any action. If a plugin just wants to signal something and has no additional data, enforcing this rule would require some dummy pointer to be sent. However, the plugin could just take action even with Data=NULL, violating the rules.
Variants for this: 1. Keep it as-is 2. Drop the supported check from docs - each plugin can offer a check if it likes 3. Introduce a constant handle, so Service("someid",QUERY_SERVICE) is the supported check
Cheers,
Udo
Udo Richter wrote:
... There's a minor taste issue that I want to bring up before its too late. The current interface suggests (but not enforces) that Service("someid",NULL) asks whether this protocol is supported or not, and doesn't cause any action. If a plugin just wants to signal something and has no additional data, enforcing this rule would require some dummy pointer to be sent. However, the plugin could just take action even with Data=NULL, violating the rules.
Variants for this:
- Keep it as-is
- Drop the supported check from docs - each plugin can offer a check if
it likes 3. Introduce a constant handle, so Service("someid",QUERY_SERVICE) is the supported check
4. Service("?someid"), maybe...
(Not "someid?", which would be the "natural" way to form a question, since "?someid" makes it simple to check for the '?').
Klaus
Udo Richter wrote:
New patch including docs etc. later this day.
... as promised. Custom plugin services, including PLUGINS.html and newplugin update. This version still mentions Data=NULL as supported query.
Cheers,
Udo
diff -au vdr-1.3.29-orig/newplugin vdr-1.3.29/newplugin --- vdr-1.3.29-orig/newplugin 2005-01-30 14:50:05.000000000 +0100 +++ vdr-1.3.29/newplugin 2005-08-20 19:45:04.000000000 +0200 @@ -170,6 +170,7 @@ virtual cOsdObject *MainMenuAction(void); virtual cMenuSetupPage *SetupMenu(void); virtual bool SetupParse(const char *Name, const char *Value); + virtual bool Service(const char *Id, void *Data = NULL); };
cPlugin${PLUGIN_CLASS}::cPlugin$PLUGIN_CLASS(void) @@ -236,6 +237,12 @@ return false; }
+bool cPlugin${PLUGIN_CLASS}::Service(const char *Id, void *Data) +{ + // Handle custom service requests from other plugins + return false; +} + VDRPLUGINCREATOR(cPlugin$PLUGIN_CLASS); // Don't touch this! };
diff -au vdr-1.3.29-orig/plugin.c vdr-1.3.29/plugin.c --- vdr-1.3.29-orig/plugin.c 2005-01-30 15:05:20.000000000 +0100 +++ vdr-1.3.29/plugin.c 2005-08-20 19:37:41.000000000 +0200 @@ -99,6 +99,11 @@ Setup.Store(Name, Value, this->Name()); }
+bool cPlugin::Service(const char *Id, void *Data) +{ + return false; +} + void cPlugin::RegisterI18n(const tI18nPhrase * const Phrases) { I18nRegister(Phrases, Name()); @@ -372,6 +377,31 @@ return NULL; }
+cPlugin *cPluginManager::CallFirstService(const char *Id, void *Data) +{ + if (pluginManager) { + for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { + cPlugin *p = dll->Plugin(); + if (p) + if (p->Service(Id, Data)) return p; + } + } + return NULL; +} + +bool cPluginManager::CallAllServices(const char *Id, void *Data) +{ + bool found=false; + if (pluginManager) { + for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { + cPlugin *p = dll->Plugin(); + if (p) + if (p->Service(Id, Data)) found=true; + } + } + return found; +} + void cPluginManager::StopPlugins(void) { for (cDll *dll = dlls.Last(); dll; dll = dlls.Prev(dll)) { diff -au vdr-1.3.29-orig/plugin.h vdr-1.3.29/plugin.h --- vdr-1.3.29-orig/plugin.h 2005-01-30 15:03:48.000000000 +0100 +++ vdr-1.3.29/plugin.h 2005-08-21 03:00:29.000000000 +0200 @@ -50,6 +50,8 @@
void RegisterI18n(const tI18nPhrase * const Phrases);
+ virtual bool Service(const char *Id, void *Data = NULL); + static void SetConfigDirectory(const char *Dir); static const char *ConfigDirectory(const char *PluginName = NULL); }; @@ -88,6 +90,8 @@ static bool HasPlugins(void); static cPlugin *GetPlugin(int Index); static cPlugin *GetPlugin(const char *Name); + static cPlugin *CallFirstService(const char *Id, void *Data = NULL); + static bool CallAllServices(const char *Id, void *Data = NULL); void StopPlugins(void); void Shutdown(void); }; diff -au vdr-1.3.29-orig/PLUGINS.html vdr-1.3.29/PLUGINS.html --- vdr-1.3.29-orig/PLUGINS.html 2005-02-12 18:08:27.000000000 +0100 +++ vdr-1.3.29/PLUGINS.html 2005-08-20 20:34:40.000000000 +0200 @@ -26,6 +26,9 @@ <!--X1.3.21--><table width=100%><tr><td bgcolor=#FF0000> </td><td width=100%> Important modifications introduced in version 1.3.21 are marked like this. <!--X1.3.21--></td></tr></table> +<!--X1.3.30--><table width=100%><tr><td bgcolor=#00AAAA> </td><td width=100%> +Important modifications introduced in version 1.3.30 are marked like this. +<!--X1.3.30--></td></tr></table> <p> VDR provides an easy to use plugin interface that allows additional functionality to be added to the program by implementing a dynamically loadable library file. @@ -68,6 +71,9 @@ <li><a href="#The Setup menu">The Setup menu</a> <li><a href="#Configuration files">Configuration files</a> <li><a href="#Internationalization">Internationalization</a> +<!--X1.3.30--><table width=100%><tr><td bgcolor=#00AAAA> </td><td width=100%> +<li><a href="#Custom services">Custom services</a> +<!--X1.3.30--></td></tr></table> <li><a href="#Loading plugins into VDR">Loading plugins into VDR</a> <li><a href="#Building the distribution package">Building the distribution package</a> </ul> @@ -866,6 +872,77 @@ and then in the global VDR texts. So a plugin can make use of texts defined by the core VDR code.
+<!--X1.3.30--><table width=100%><tr><td bgcolor=#00AAAA> </td><td width=100%> +<a name="Custom services"><hr><h2>Custom services</h2> + +<center><i><b>What can I do for you?</b></i></center><p> + +In some situations, two plugins may want to communicate directly, talking about things +that VDR doesnt handle yet as mediator. For example, a plugin may want to use features +that a different plugin offers, or a plugin wants to inform other plugins about important +things it does. To receive requests or messages, a plugin can implement the +following function: + +<p><table><tr><td bgcolor=#F0F0F0><pre> +virtual bool Service(const char *Id, void *Data = NULL); +</pre></td></tr></table><p> + +<tt>Id</tt> is an unique identification string that identifies the service protocol. +To avoid collisions, the string should contain a service name, the plugin name (unless +the service is not related to a single plugin) and a protocol version number. +<tt>Data</tt> points to a custom data structure or is <tt>NULL</tt> to detect whether +the plugin supports this service. For each id string there should be a specification +that describes the format of the data structure, and any change to the format should +be reflected by a change of the id string. +<p> +The function shall return true for any service ID string it handles, and false +otherwise. The function shall not perform any actions as long as <tt>Data</tt> is +<tt>NULL</tt>. The plugins have to agreee in which situations the service +may be called, for example whether the service may be called from every thread, or +just from the main thread. A possible implementation could look like this: + +<p><table><tr><td bgcolor=#F0F0F0><pre> +struct Hello_SetGreetingTime_v1_0 { + int NewGreetingTime; +}; + +bool cPluginHello::Service(const char *Id, void *Data) +{ + if (strcmp(Id, "Hello-SetGreetingTime-v1.0") == 0) + { + if (Data == NULL) return true; + GreetingTime = ((Hello_SetGreetingTime_v1_0*)Data)->NewGreetingTime; + return true; + } + return false; +} +</pre></td></tr></table><p> + +<p> +To send messages to, or request services from a specific plugin, the plugin can directly call its +service function: + +<p><table><tr><td bgcolor=#F0F0F0><pre> +Hello_SetGreetingTime_v1_0 hellodata; +hellodata.NewGreetingTime = 3; +cPlugin *Plugin = cPluginManager::GetPlugin("hello"); +if (Plugin) + Plugin->Service("Hello-SetGreetingTime-v1.0", &hellodata); +</pre></td></tr></table><p> + +To send messages to, or request services from some plugin that offers the protocol, the +plugin can call the function <tt>cPluginManager::CallFirstService</tt>. This function +will send the request to the first plugin that supports this service protocol. The +function returns a pointer to the plugin that handled the request, or <tt>NULL</tt> +if no plugin handles the request. +<p> +To send a messages to all plugins, the plugin can call the function +<tt>cPluginManager::CallAllServices</tt>. This function will send the request to +all plugins that support this service protocol. The function returns <tt>true</tt> if +any plugin handled the request, or <tt>false</tt> if no plugin handled the request. + +<!--X1.3.30--></td></tr></table> + <a name="Loading plugins into VDR"><hr><h2>Loading plugins into VDR</h2>
<center><i><b>Saddling up!</b></i></center><p>
Udo Richter wrote:
Udo Richter wrote:
New patch including docs etc. later this day.
... as promised. Custom plugin services, including PLUGINS.html and newplugin update. This version still mentions Data=NULL as supported query. ... +To send messages to, or request services from some plugin that offers the protocol, the +plugin can call the function <tt>cPluginManager::CallFirstService</tt>. This function +will send the request to the first plugin that supports this service protocol. The
Actually it sends the call to _every_ plugin and returns as soon as one has processed it. Should this be rephrased, or should the implementation be changed to
cPlugin *cPluginManager::CallFirstService(const char *Id, void *Data) { if (pluginManager) { for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { cPlugin *p = dll->Plugin(); if (p && p->Service(Id, NULL) && p->Service(Id, Data)) return p; } } return NULL; }
+function returns a pointer to the plugin that handled the request, or <tt>NULL</tt> +if no plugin handles the request. +<p> +To send a messages to all plugins, the plugin can call the function +<tt>cPluginManager::CallAllServices</tt>. This function will send the request to +all plugins that support this service protocol.
Actually it sends the request to _all_ plugins, because it doesn't explicitly check whether a particular plugin supports it. In this case this doesn't make much difference, but I think the description should reflect what actually happens.
Klaus
Klaus Schmidinger wrote:
Actually it sends the call to _every_ plugin and returns as soon as one has processed it. Should this be rephrased, or should the implementation be changed to
if (p && p->Service(Id, NULL) && p->Service(Id, Data)) return p;
I would prefer to not call the function twice, to avoid the redundant string compare.
----8<---- To send messages to, or request services from some plugin that offers the protocol, the plugin can call the function <tt>cPluginManager::CallFirstService</tt>. This function will send the request to all plugins until one plugin handles the service call. The function returns a pointer to the plugin that handled the call, or <tt>NULL</tt> if no plugin handled the call. <p> To send a messages to all plugins, the plugin can call the function <tt>cPluginManager::CallAllServices</tt>. The function returns <tt>true</tt> if any plugin handled the service call, or <tt>false</tt> if no plugin handled the call. ----8<----
Cheers,
Udo
Klaus Schmidinger wrote:
Udo Richter wrote:
Variants for this:
- Keep it as-is
- Drop the supported check from docs - each plugin can offer a check if
it likes 3. Introduce a constant handle, so Service("someid",QUERY_SERVICE) is the supported check
- Service("?someid"), maybe...
cwieninger@gmx.de wrote:
IMHO 3. is the best solution.
Since I don't want to enforce extra supported checks, I tend to a variation of 2: Suggest to use NULL as install check without action. Enforcing an install check in docs and not using it is inconsistent.
----8<---- <tt>Data</tt> points to a custom data structure. For each id string there should be a specification that describes the format of the data structure, and any change to the format should be reflected by a change of the id string. <p> The function shall return true for any service ID string it handles, and false otherwise. The plugins have to agreee in which situations the service may be called, for example whether the service may be called from every thread, or just from the main thread. Plugins can implement a 'service supported' check by returning <tt>true</tt> without performing any actions when called with <tt>Data=NULL</tt>. ----8<----
Cheers,
Udo
Udo Richter wrote:
Klaus Schmidinger wrote:
Udo Richter wrote:
Variants for this:
- Keep it as-is
- Drop the supported check from docs - each plugin can offer a check if
it likes 3. Introduce a constant handle, so Service("someid",QUERY_SERVICE) is the supported check
- Service("?someid"), maybe...
cwieninger@gmx.de wrote:
IMHO 3. is the best solution.
Since I don't want to enforce extra supported checks, I tend to a variation of 2: Suggest to use NULL as install check without action. Enforcing an install check in docs and not using it is inconsistent.
----8<---- <tt>Data</tt> points to a custom data structure. For each id string there should be a specification that describes the format of the data structure, and any change to the format should be reflected by a change of the id string.
<p> The function shall return true for any service ID string it handles, and false otherwise. The plugins have to agreee in which situations the service may be called, for example whether the service may be called from every thread, or just from the main thread. Plugins can implement a 'service supported' check by returning <tt>true</tt> without performing any actions when called with <tt>Data=NULL</tt>. ----8<----
Since the Data parameter in the function calls has NULL as default, a plugin implementing Service() _must_ be prepared to handle this case. This text makes it look like this is somehow optional and up to the individual plugin. If you want it that way, then we'll have to remove the NULL default parameter. Personally I'd rather keep it as it is right now.
Klaus
Klaus Schmidinger wrote:
from every thread, or just from the main thread. Plugins can implement a 'service supported' check by returning <tt>true</tt> without performing any actions when called with <tt>Data=NULL</tt>. ----8<----
Since the Data parameter in the function calls has NULL as default, a plugin implementing Service() _must_ be prepared to handle this case.
----8<---- Plugins should expect to be called with <tt>Data=NULL</tt> and may use this as a 'service supported' check without performing any actions. ----8<----
Cheers,
Udo
-
Christian Wieninger -
Clemens Kirchgatterer -
cwieninger＠gmx.de
-
Klaus Schmidinger -
Martin Wache -
Udo Richter