search

beyondscreen / node-rpi-ws281x-native

native bindings to drive WS2811 (or WS2812) LED-Controllers on a Raspberry Pi
MIT License
224 stars 101 forks source link

Throw error and/or return mock. #42

Closed Guuz closed 8 years ago

Guuz commented 8 years ago

Hello,

I'm using this module in my app and stumbled upon the following. During development I don't want to run as root (why should i?) and I cant really use this module anyway because it only works on a Pi. The code here logs to strerr and exits the process. This is not very friendly for a consumer.

I would propose we throw an error that the consumer can detect and ignore during development. Maybe the mock implementation could be made available to use too.

Curious to hear what your opinion is on this! I can work on a PR if we come to a solution.

usefulthink commented 8 years ago

I like that Idea. I never ran into such a situation myself, but it makes complete sense to be a bit more friendly about that :) I would suggest to move the root-verification into the getNativeBindings-function, similar to what is happening with the raspberry-detection.

Guuz commented 8 years ago

getNativeBindings() makes sense yes! I think it's very bad for a 3th party lib to call process.exit().

usefulthink commented 8 years ago

You are right. Would be better to just throw an exception or handle it gracefully (like it is currently done for non-pi architectures). The consuming library will then either have to catch and handle it or will die anyway with an uncaught exception. Would be great of you could create a PR for this if you find the time.

Guuz commented 8 years ago

I'm on it!

Guuz commented 8 years ago

@usefulthink I changed the implementation a bit compared to what we talked about here. But i think it's better this way. I hope you agree! If not I'm happy to change it.