search

node-dmx / dmx

DMX controller library for node.js
MIT License
297 stars 96 forks source link

Added support for defining devices in config file #64

Closed spearway closed 5 years ago

spearway commented 6 years ago

This enables the definition of new devices in the config file making it easier to test new devices.

spearway commented 6 years ago

https://github.com/wiedi/node-dmx/issues/50

Fensterbank commented 5 years ago

@spearway I don't really understand you're changes in dmx.js. This change indmx-web.js makes sense, because you now pass the config file to the DMX constructor in the web ui. https://github.com/wiedi/node-dmx/pull/64/files?utf8=%E2%9C%93&diff=unified#diff-c9e5ae1e07767abf86038db8a691fed8R26

But the changes in dmx.js aren't clear for me, there is no real functional change, instead anim.js is required two times now and is available with DMX.Animation and new DMX().animation. I don't see the point.

spearway commented 5 years ago

The purpose is to ensure that we can override the device definition in the config file but that we do not remove any existing one doing this. In practice we can augment the device definition or override it but any existing device definition in devices is always there.

There may be an easier way of doing this but it is the intent.

Fensterbank commented 5 years ago

Thanks for explanation. Seems good to me. Merged. Thanks! 🍪