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 an additional source for converter configuration #232

Closed
danielwelch opened this issue Jul 30, 2018 · 7 comments
Closed

Add an additional source for converter configuration #232

danielwelch opened this issue Jul 30, 2018 · 7 comments
Labels
feature request Feature request stale Stale issues

Comments

@danielwelch
Copy link
Contributor

danielwelch commented Jul 30, 2018

Updating and using zigbee-shepherd-converters as a seperate node module seems to be working pretty nicely, but it causes a problem for add-on users as editing the devices file requires connecting to the add-on's docker container, which is confusing for some people (see discussion here).

I don't want to rock the boat too much just for this use case, but maybe it'd be possible to add an additional file to "check" for configuration of additional devices. If there was, the add-on could expose a .js file in the share directory, which would make it easy for add-on users to add unsupported devices in a way that they're used to.

@danielwelch
Copy link
Contributor Author

My basic thought process for how this would work is to add a configuration.yaml option localDevices (or whatever) that defines a module path and do something along the lines of:

// some psuedo-code
var zigbeeShepherdConverters = require('zigbee-shepherd-converters');
if (settings.get().localDevices) {
    const localDevices = require(settings.get().localDevices);
    zigbeeShepherdConverters.devices = { ...localDevices, ...zigbeeShepherdConverters.devices };
}

and require the user to implement a module at the path that exports a devices object.

@Koenkk
Copy link
Owner

Koenkk commented Jul 31, 2018

Yes, that would be a nice solution, however we also need to import the fromZigbee and toZigbee from that path.

@nic0dk
Copy link

nic0dk commented Jul 31, 2018

+1 here,
do have a few zigbee device i need to add support for and are using Daniel's addon.
BTW awesome project you 2 guys made - Thanks!

@tb-killa
Copy link
Contributor

What about building converter per device as module files.
We provide for every device typ one single file and do some sort of recursive files on folder search on zigbee2mqtt startup and include them.
With this everyone could build own converter and doesn´t have to handle with modified ...

@Koenkk
Copy link
Owner

Koenkk commented Aug 27, 2018

Yes that would be nice (that's also how smartthings solves it), but this would require a complete refactor of zigbee-shepherd-converters, maybe one separate file is enough for now?

@danielwelch
Copy link
Contributor Author

Yes, that would be a nice solution, however we also need to import the fromZigbee and toZigbee from that path.

Can you give me some more detail here? I’m not sure how this would work as the converters aren’t exposed like devices are (or, they’re only exposed as a prop of devices). What if a device defined by a user needs a converter defined in zigbee-shepherd-converters? Or is that not a concern?

I’d like to implement this and open a PR as it seems a lot of issues keep arising from this, and I’d like to help.

@stale
Copy link

stale bot commented Dec 15, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale issues label Dec 15, 2018
@stale stale bot closed this as completed Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request stale Stale issues
Projects
None yet
Development

No branches or pull requests

4 participants