Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plugin methods to smarthome #50

Closed
wants to merge 9 commits into from

Conversation

ohinckel
Copy link
Member

(copied from mknx/smarthome#81)

This patch will add the following function to smarthome instance:

  • return_plugin(name) - returns the plugin instance for given plugin name
  • get_plugin_ident(instance) - returns the thread identifier of the given plugin instance, which should be unique
  • get_plugin_name(instance)' - returns the plugin name (which is the section name inplugin.conf`)

These function may help when running multiple instances of one plugin and each want to add a scheduler task. Usually you do something like sh.scheduler.add("PluginName", ...). But when registering a plugin multiple times, only one scheduler task will be registered.

E.g. having something like this in plugin.conf:

[name1]
  class_name = ThePlugin
  class_path = plugins.theplugin
  option = somevalue
[name2]
  class_name = ThePlugin
  class_path = plugins.theplugin
  option = othervalue

In this case the plugin is registered two times (one with sh.name1 and another one with sh.name2). When this plugin is registering a scheduler task, usually it will do something like this sh.scheduler.add("ThePlugin", ...) - but this will only register one scheduler task, since the other will overwrite the previous scheduler task because it uses the same name!

To solve this problem, the plugin can use the thread identifier and can do something like this:
sh.scheduler.add("ThePlugin-" + str(sh.get_plugin_name(self)), ...), which will register two scheduler tasks now since both includes the unique thread identifier in the scheduler task.

I don't know of any other solution how to configure a plugin multiple times (e.g. for different devices) and having a scheduler task for each of them.

This will also made the SML plugin able to register multiple times (e.g. when having multiple power meter devices or serial devices connected).

  and get_plugin_ident() to return the thead ident for a given plugin
  instance
… register

  this plugin multiple times
# e.g. when having more than one power meter device and serial devices
…trieve the plugin name

  for given plugin instance
# can also be used instead of plugin ident (which is the thread`s identifier) for
# better readability
…eduler with

  the plugin name instead of plugin ident, which can not be used to identify the
  plugin instance
@ohinckel
Copy link
Member Author

@i-am-offline You added some methods to retrieve plugin instances (see mknx/smarthome#181) which seems to be replaced with the implementation of this PR.

Do you think you can update your implementation and create a new PR here when this PR will be merged?

@thernst-de
Copy link
Contributor

Yes, I can do so. But it can take some weeks until I find the time for this.

@psilo909
Copy link
Contributor

I could use that PR for my two Plugins (Enigma2, AVM) pretty well. But i am not sure about conflicts in case i merge it.
@ohinckel: u see any risks? what are i-am-offline's modifications? do we need his rewriting

@cstrassburg
Copy link
Member

there are no risks. this methods are new, but there is an other solution in knx plugin pulled by @encbladexp see #22
It's time for a homogeneous implementation.

@cstrassburg
Copy link
Member

cstrassburg commented May 16, 2016

In my opinion this:
'-' + str(self._sh.get_plugin_name(self))
can be also implemented inside the scheduler, so every plugin definition in the conf get its own scheduler tasks by default.
Either by adding a new parameter to:
def add(self, name, obj, prio=3, cron=None, cycle=None, value=None, offset=None, next=None):

or maybe it is possible to get this information from obj.

@ohinckel
Copy link
Member Author

ohinckel commented May 16, 2016

AFAIS @encbladexp implementation adds only a instance parameter (e.g. for logging) and an option knx_instance to items to attach them to a given instance. Both can be done by using stuff from this PR (e.g. for logging one could use the get_plugin_name()) and another PR not created (e.g. for instance assignment to items use the plugin setting - see mknx/smarthome#83 which should also be merged; I will add it to #52 ).

A default scheduler task does not make sense, since a plugin may require multiple scheduler tasks - or no task at all. And I would provide such functionality (e.g. adding a task) within a base class which could be used by all plugins instead of letting them use the scheduler directly. Further this base class implementation could make use of the menionted PRs.

@cstrassburg
Copy link
Member

cstrassburg commented May 16, 2016

ich switche mal die Sprache.
Von einem default scheduler Task war keine Rede, sondern ein default Verhalten, beim hinzufügen eines tasks. Wenn bei jedem scheduler.add der Pluginname zum Namen hinzugefügt wird, dann hätte man eine saubere Trennung pro Plugin.
Aus Task "task1" des Plugins "plug1" wird dann "plug1-task1"
Aus Task "task2" des Plugins "plug1" wird dann "plug1-task2"

bei einer zweiten Instanz des Plugins würde das dann so aussehen:
Aus Task "task1" des Plugins "plug2" wird dann "plug2-task1"
Aus Task "task2" des Plugins "plug2" wird dann "plug2-task2"

@cstrassburg
Copy link
Member

Diese Art der Implementierung ist etwas unschön, zu viel redundanter Code und nicht objektorientiert:

smarthome.py

 def return_plugin(self, name):
        return self._plugins.get_plugin(name)

 def get_plugin_ident(self, plugin):
        return self._plugins.get_plugin_ident(plugin)

 def get_plugin_name(self, plugin):
        return self._plugins.get_plugin_name(plugin)

plugin.py

 def get_plugin(self, name):
        for thread in self._threads:
            if thread.name == name:
               return thread.plugin
        return None

 def get_plugin_ident(self, plugin):
        for thread in self._threads:
            if thread.plugin == plugin:
               return thread.ident
        return None

 def get_plugin_name(self, plugin):
        for thread in self._threads:
            if thread.plugin == plugin:
               return thread.name

Ich würde da eher den objektorientierten Ansatz bevorzugen.

smarthome.py

 def return_plugins(self):
        return self._plugins

plugin.py in class plugins

 def get_plugin(self, name):
        for thread in self._threads:
            if thread.name == name:
               return thread.plugin
        return None

plugin.py in class PluginWrapper

def get_ident(self):

def get_name(self):

Im Code wäre es dann so zu verwenden:

self._sh.get_plugins().get_plugin(self).get_name()

@ohinckel
Copy link
Member Author

@cstrassburg danke fuer dein Feedback. Ich kann das gerne so anpassen und den PR entsprechend aktualisieren. War schon >2 Jahre her, als ich den Original-PR erstellt habe und gerade ich mich fuer ca. 4 Monate mit SmartHome.py beschaeftigt hatte 😉 Aber hier bekommt man wenigstens auch mal Feedback, was die letzten zwei Jahre ausgeblieben ist.

@cstrassburg
Copy link
Member

Es geht voran!
Ich implementiere das mal, schreibe gleich die Doku und erstelle nen Testcase dazu, dann brauchst du den PR nicht anzufangen.

@cstrassburg
Copy link
Member

implementiert

@ohinckel ohinckel deleted the dev-plugin-ident branch December 23, 2016 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants