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

Plugin system #293

Merged
merged 10 commits into from Nov 3, 2015
Merged

Plugin system #293

merged 10 commits into from Nov 3, 2015

Conversation

nfarina
Copy link
Contributor

@nfarina nfarina commented Oct 19, 2015

Proposal for new "Plugin" system.

This splits Homebridge into a "core" npm package with minimal dependencies that is intended to be installed globally via a single command npm install -g homebridge.

By itself, Homebridge does nothing - you must install one or more Plugins to have any functionality.

Plugins are published separately to npm and installed globally like npm install -g homebridge-lockitron.

Since homebridge will now be system-global, all user files have been moved to $HOME/.homebridge. For now, that means config.json and persist are epected to be there instead of inside the Homebridge package.

Plugins can "register" one or more accessories or platforms by calling methods on the homebridge API object passed to the new "plugin initializer" method which is expected to be the root export of the plugin (this is similar to Grunt plugins).

See example-plugins/homebridge-lockitron for an example of a Plugin package.

All of today's accessories and platforms would be very slightly modified to access HAP classes via the homebridge API, and temporarily published to a new npm package homebridge-legacy. So today's users could type npm install -g homebridge-legacy and get all the accessories and platforms as they are today.

However, accessory and platform authors will be strongly encouraged to create their own Github repos and npm packages for their own Plugins, so that users can install only what they need, and are in control of when to upgrade plugins. This will also naturally move the very high amount of issue/PR traffic to the Plugin repositories where they belong.

I'm still working on porting the existing accessories+platforms to the new homebridge-legacy repo, but figured I would make this PR for discussion in the meantime.

/cc @cflurin, @snowdd1, @SphtKr, @maddox, @madmod, @KhaosT
Continuing discussion from #278

  - Homebridge is now designed to be `npm install`d globally and
executed via "homebridge" script
  - Remove all specific accessories/platforms except for an example
  - New internal structure and "cli"
  - Homebridge calls single exported initializer function and passes an
API object
  - No more require() for HAP classes (doesn't play well with plugin
structure)
@KhaosT
Copy link
Contributor

KhaosT commented Oct 19, 2015

This is amazing :) Thanks for doing all of these. As you mentioned that homebridge will now be installed globally via npm, would you mind to share the reason why you choose to do so since it suppose to be an end application (instead of... a library)?

@cflurin
Copy link
Contributor

cflurin commented Oct 19, 2015

@farina: I was working on a solution based on "child_process" in order to install npm-modules but it's now obsolete. I like your plugin-approach. BTW I hosted my modest Accessory Shims on a separate GitHub from the beginning for the reasons you mentioned above.

@justme-1968
Copy link
Contributor

the plugin part and the possibility to have the plugins in different repositories sounds great.

for a global install the config file should also go to a global location not to a home directory and it would be nice to have the option to still install local.

@snowdd1
Copy link
Contributor

snowdd1 commented Oct 19, 2015

Great work!
I am still a little puzzeled about everything installed globally.

And I am a slow reader, so I did not yet fully understand how the plugins are supposed to register themselves. Does the plugin.js search all known paths and tries to load them, or do you only search for them if they are encountered in config.json?
If the first, all plugins that have once been installed by the user have to stay current (and are not updated automatically from the npm repositories); if the latter I simply didn't find it in the code, but I would prefer loading only if required.

However, I will try this next weekend and try to update my homebridge-knx repo to match the new API for testing!

@nfarina
Copy link
Contributor Author

nfarina commented Oct 19, 2015

So I think the most user-friendly "ideal" would be for Homebridge to be an application downloaded and installed the "normal way" according to each platform, and for plugins to be downloaded, updated, and maintained programmatically by Homebridge in the user's home directory (or anywhere), just like the standalone node-based application Atom.

I spent a little time going down this road and it's a long path; it requires an extensive command-line interface or a web UI, and although I started on both of those projects, I simply ran out of time.

This "grunt style" system I believe is the next-best option that gives us the most bang for our buck in terms of time spent reinventing the wheel. We get to leverage npm for downloading stuff, versioning, updating, and peer dependencies. Npm states that the global folder is to be used for "command-line applications" and this is essentially what homebridge is today. Once installed globally, you can simply type homebridge and it will start up and automatically discover other plugins installed globally (see Plugin.installed()).

Updating plugins is easy for the user as well: npm install -g homebridge-lockitron.

You could install homebridge+plugins anywhere else if you wish - download it from Github or install it locally somewhere - all that matters is that your plugins are either in your local node_modules or you can simply pass your plugin directory as a command-line argument to homebridge.

@snowdd1
Copy link
Contributor

snowdd1 commented Oct 19, 2015

it requires an extensive command-line interface or a web UI

Don't we mix two separate issues here? We have the config.json text-based configuration, which, esp. for the JSON illiterate like me, is very error prone. For that, a web-based configuration with validated input fields and nice check boxes would be a great thing - But IMHO this is only little related to installing plugins based on data in that very config.json
Why can't we simly npmi.install() an accessory or a platform that we find in the config.json? Of course that's as error prone as any typo in config.json. I tried that as a testing case in my repo and it worked sufficiently well with my platform.

Nevertheless, I'll try to update my platform's repository next weekend to match your plugin-system branch's API.

@cflurin
Copy link
Contributor

cflurin commented Oct 20, 2015

I've installed the new homebridge (Plugin system branch) on a Mac mini. My first Plugin FakeOutlet works as expected.

@cflurin
Copy link
Contributor

cflurin commented Oct 20, 2015

... I forgot to mention: I had to remove a comma in homebridge-lockitron > index.js

...
    }
  }.bind(this));
}

LockitronAccessory.prototype.getServices = function() {
  return [this.service];
}

@nfarina
Copy link
Contributor Author

nfarina commented Oct 20, 2015

Fixed the comma, thanks! FakeOutlet looks great, that's awesome you got it to work :)

@BerndDA
Copy link

BerndDA commented Oct 20, 2015

Hi,
there seems to be an error in server.js line 179. should be:
var platformLogger = Logger.withPrefix(platformName);
Additionally "once" in line 190 causes an error. But this might be caused by my setup...
Regards
Bernd

@nfarina
Copy link
Contributor Author

nfarina commented Oct 20, 2015

@BerndDA Thanks! Fixed the logger. The once error is probably due to the platform you're using - I think at least one platform (incorrectly) calls the callback more than once. Which platform are you using?

@BerndDA
Copy link

BerndDA commented Oct 20, 2015

Hi.
basically I get a compile error:
server.js:190
platformInstance.accessories(once(function(foundAccessories){
^
ReferenceError: once is not defined
But as I said it migt be caused by my setup.
Currently porting my own platform that is written in typescript and compiled on windows using VS.
It is more an educational project for myself to learn node & typescript, cause there already is a public implementation of the platform available.

@nfarina
Copy link
Contributor Author

nfarina commented Oct 20, 2015

@BerndDA Ah ha, now I'm with you. Should be fixed now!

@cflurin
Copy link
Contributor

cflurin commented Oct 20, 2015

@farina: migrating to the Plugin system might be the opportunity to discuss some wishes.

  1. What about passing the hab object instead of define it in every Plugin? The Plugin should not directly depend on hab-nodejs. I'm not sure about this point, because you already modified it in the module.exports function, wich is better than define var Service = require("hap-nodejs").Service;
  2. It would be also useful to be able to access the full config.json.

e.g. for the accessories (server.js):

- var accessoryInstance = new accessoryConstructor(accessoryLogger, accessoryConfig);
+ var accessoryInstance = new accessoryConstructor(hab, config, accessoryLogger, accessoryConfig);

Instead of accessoryConfig the accessory index will do the job as well.

@nfarina
Copy link
Contributor Author

nfarina commented Oct 20, 2015

@cflurin Check out the Lockitron Sample in the plugin branch, it passes the hap-nodejs inside the new homebridge API object so that plugins need not depend on HAP-NodeJS directly.

It seems like we could add the full config.json to the homebridge API object as well - what would you use that for?

@cflurin
Copy link
Contributor

cflurin commented Oct 21, 2015

@nfarina passing the hap-nodejs to the plugin is perfect.
Concerning the config.json I'd like to define global parameters like ip-address, port, authname, password etc.

As you proposed I successfully tried this solution:

api.js:

function API() {
...
 + this.globalConfig;

server.js:

function Server() {
  this._api = new API(); // object we feed to Plugins
-  this._plugins = this._loadPlugins(); // plugins[name] = Plugin instance
  this._config = this._loadConfig();
  this._bridge = this._createBridge();
+  this._api.globalConfig = this._config.global;
+  this._plugins = this._loadPlugins(); // plugins[name] = Plugin instance
}

plugin > index.js:

+ var globalConfig;

module.exports = function(homebridge) {
  Service = homebridge.hap.Service;
  Characteristic = homebridge.hap.Characteristic;
+  globalConfig = homebridge.globalConfig;

  homebridge.registerAccessory("FakeOutlet", FakeOutletAccessory);
}

function FakeOutletAccessory(log, config) {

...
  var url = globalConfig.url;
...

config.json example:

{
    "global": {
        "url": "127.0.0.1",
        "port": "8083",
        "auth": {"user": "user", "password": "password"}
    },
    "bridge": {
        "name": "Homebridge-P",
        "username": "CC:22:3D:E3:CE:30",
        "port": 51826,
        "pin": "031-45-154"
    },

    "platforms": [],               

    "accessories": [
        {
            "accessory": "FakeOutlet",
            "name": "Fake Outlet"
        }
    ]
}

@SphtKr
Copy link
Contributor

SphtKr commented Oct 22, 2015

I'm not sure I agree that what @cflurin has above is a good design, but it raises the question: under a modularized design, should all the modules share a config.json at all? Or should we all have our own? Is this a good way to do it or is it just the way it was done? KNX already has its own, I think.

The minimum necessary information in the shared config.json is for Homebridge to know that the plugin should be initialized--can it detect the fact that a plugin has been installed another way? And even if config.json is required to have this information, should the configuration information for individual modules now no longer be in a shared config file? I'm probably in favor of having separate config files per plugin. If a "suite" of plugins needs to share config information, that can be a design choice of the suite developer where and how to store the config.

@nfarina
Copy link
Contributor Author

nfarina commented Oct 22, 2015

Ultimately I would like configuration to be done through a web UI, and for homebridge to abstract the storage of configuration details away from plugins. Plugins shouldn't have to know that config.json exists or what the format is.

For now though this is what we have. And I'm not sure I would want plugins to have access to the full config because different plugins may have different ideas about what goes in the "global" area.

I think the underlying desire is to have plugin-specific or platform-specific config (or both) that can be accessed by all accessories in a plugin. Would that help @cflurin?

@justme-1968
Copy link
Contributor

@cflurin wants to have a common section/config per group of accessories. his accessories are independent but access a common backend. so a part of the config is identical.

i think a single global section like this will create conflicts as there is no way to specify for which plugin/plugins an entry should be valid. what about two completely different plugins wanting to use global.url or global.port. this will not work.

defaults should be valid only for a group of accessories or platforms and there should be multiple groups allowed.

but i will get nested quite a bit and is not very well suited for manual editing.

@cflurin
Copy link
Contributor

cflurin commented Oct 22, 2015

I agree we should define the plugin parameters outsite the homebridge config.json. My solution using golbalConfig was just to simplify the configuration based on the actual homebridge design during the developing process.
My goal is a module with one homebridge interface (API) that ideally shouldn't change in the future. So this again is the opportunity to define a possible final homebridge API.

The sample above using configGlobal is simplified, I wanted to create a "section" to define some global parameters outsides bridge, platforms and accessories. I don't see any conflicts if it is used properly.
"global" is just a section reserved for the plugin-definitions.

e.g.

{
    "global": {
        "fhem-server": {
            "url": "127.0.0.1",
            "port": "8083",
            "auth": {
                "user": "user",
                "password": "password"
            }
        },
        "zwave-sever": {
            "url": "127.0.0.1",
            "port": "8083"
        },
        "enoceon": {
            "url": "127.0.0.1",
            "port": "8083"
        }
    },
    "bridge": {
        "name": "Homebridge-P",
        "username": "CC:22:3D:E3:CE:30",
        "port": 51826,
        "pin": "031-45-154"
    },
    "platforms": [],
    "accessories": [
        {
            "accessory": "FhemSwitch",
            "name": "at_home"
        },
        {
            "accessory": "FhemSwitch",
            "name": "blind_control"
        },
        {
            "accessory": "FhemSwitch",
            "name": "alarm"
        }
    ]
}

As you can see this way I can start several instances of the same shim (plugin) just defining another name. Sure there are other ways to implement this feature and as I said this isn't my final solution.

@cflurin
Copy link
Contributor

cflurin commented Oct 22, 2015

@nfarina: I don't need globalConfig anymore, I solved it with a separate config.json.

I've locally installed homebridge in a folder homebridge-p, the plugins are placed in the subfolder node_modules.
I noticed that all the plugin modules in this folder are loaded by homebridge. Is there another way to detect which plugins should be loaded? The point is that a user/developer might place several plugins in the folder node_modules but would like to only use some of them.

@nfarina
Copy link
Contributor Author

nfarina commented Oct 22, 2015

@cflurin If you want to control which plugins are loaded, you could move them out of the node_modules belonging to homebridge and put them somewhere else - then pass the command-line argument --plugin-path to the homebridge cli and point to a directory containing the plugins you would like to load.

@KhaosT
Copy link
Contributor

KhaosT commented Oct 22, 2015

To leave the space for future configuration from web/app, do you think we should define an interface to get required config parameters on the plugin side so that if we want to allow modifying configuration without manually editing config.json in the future, the existing plugins will support this automatically without any modification?

@cflurin
Copy link
Contributor

cflurin commented Oct 22, 2015

@nfarina:
okey, probably it's only necessary for development / experimenting.

@KhaosT:
Basically a config.json is a file that can be edited or programmatically modified. In a way this is also an interface. Actually I've already defined parameters in the homebridge config.json and used them opening the file from my application (shim). But for the new API I think the homebridge config.json should only define parameters concerning homebridge. Maybe knowing the config-path in the plugin could be useful in the future.

@snowdd1
Copy link
Contributor

snowdd1 commented Oct 23, 2015

KNX already has its own

No, currently the KNX platform shim is using the config.json of homebridge - I made it a platform (though it does not support auto discovery and such) to have a common section for all accessories of that type. In my first version the platform part was just a loader for a set of knx accessories, loaded depending on the accessory type in config.json (in the KNX platform section).

I will move the platform configuration to a separate file, to - in the long run - have a web form based configuration tool for that, based on accessory definition templates and supporting value validation (for KNX addresses e.g.).

If homebridge gets its own web server, platforms should be able to pass a link to their configuration web page back to homebridge, so homebridge's web page can lists links to the installed platforms, for individual configuration of those.

@ilcato
Copy link
Contributor

ilcato commented Nov 3, 2015

@nfarina Plugin updated

@nfarina
Copy link
Contributor Author

nfarina commented Nov 3, 2015

Alright, let's do this. I'm ready to make the split. Most plugin-related PRs are closed right now so today will be the day.

@KhaosT
Copy link
Contributor

KhaosT commented Nov 3, 2015

@nfarina Awesome :)

@nfarina
Copy link
Contributor Author

nfarina commented Nov 3, 2015

@snowdd1 @ilcato @cflurin Could you guys update your repos on npm to add the plugin name as the first parameter to homebridge.registerAccessory and/or homebridge.registerPlatform? This is needed to prevent conflicts especially when using the new legacy shims plugin.

Example: https://github.com/nfarina/homebridge/blob/plugin-system/example-plugins/homebridge-lockitron/index.js#L8

@nfarina
Copy link
Contributor Author

nfarina commented Nov 3, 2015

Alright, just published the new homebridge to npm as v0.2.0. Here we go!

nfarina added a commit that referenced this pull request Nov 3, 2015
@nfarina nfarina merged commit 3e7d2ae into master Nov 3, 2015
@nfarina nfarina deleted the plugin-system branch November 3, 2015 23:22
@KhaosT
Copy link
Contributor

KhaosT commented Nov 3, 2015

🎉👏😊

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

@nfarina: great! just updated npm and github.

@ilcato
Copy link
Contributor

ilcato commented Nov 4, 2015

@nfarina: very good! Updated npm and github.

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

I just tried to install homebridge on a RPi:

npm install -g homebridge

But it seems to install an old version 0.1.1.

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

... the github-homebridge version works properly:

git clone https://github.com/nfarina/homebridge.git
cd homebridge
sudo npm install -g

@ilcato
Copy link
Contributor

ilcato commented Nov 4, 2015

Installed homebridge with npm on a Mac and on a rpi2 without problem. Version 0.2.1 installed correctly.

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

@ilcato: strange, I did the same on a Mac and also on a RPI 2. I'm investigating ...

@ilcato
Copy link
Contributor

ilcato commented Nov 4, 2015

@cflurin try to uninstall the module before installing again.

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

yes, I did uninstall and clean without success

sudo npm uninstall -g homebridge
sudo npm cache clean

@nfarina
Copy link
Contributor Author

nfarina commented Nov 4, 2015

that is super weird. are you running this from inside a directory with a package.json in there? It shouldn't matter though...

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

@nfarina: I tried all the time in the home dir (no package.json).
But I successfully installed the latest Version 0.2.3 on the RPi. I really don't know the reason.
I'll try now on the Mac.

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

... just now I can only install the latest version of homebridge locally.

sudo npm install homebridge

if I try to install homebridge globally, version 0.1.1 is installed!
Any suggestions?

@ilcato
Copy link
Contributor

ilcato commented Nov 4, 2015

what version of npm do you use?

@cflurin
Copy link
Contributor

cflurin commented Nov 4, 2015

Mac: node: 0.12.7, npm: 2.11.3
RPi 2 (Jessie): node: 4.2.1, npm: 3.3.10

@ilcato
Copy link
Contributor

ilcato commented Nov 4, 2015

I'm both on Mac and RPi 2 with:
node: 4.2.1, npm: 3.3.10
and I have no problem.

@snowdd1
Copy link
Contributor

snowdd1 commented Nov 5, 2015

@nfarina : I just published 0.2.0 version of homebridge-knx that matches the API change and the package.json requirements.
You might want to remove the KNX part from the legacy plugin. It's all files starting with KNX or knx.
Thanks & regards
Raoul

@nfarina
Copy link
Contributor Author

nfarina commented Nov 5, 2015

@snowdd1 Awesome, done!

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

8 participants