Closed gbkwiatt closed 3 years ago
Had to comment out rpi-version as it does not work with RPI 4
First of all, thanks a lot for your contribution - as I didn't find a lot of time to work on this, any help here is much appreciated.
I have a few questions about this though:
would you mind keeping the package-name and all the URLs in the package.json as they are? I have absolutely no problem with you maintaining a fork (if you are interested, we can even discuss getting you to be a maintainer of the main repo somehow), but for the main repo I'd like to keep them as they are.
about removing the hardware version-checks, I think they can go away entirely as the upstream-library now makes sure that it doesn't blow up on unsupported hardware: https://github.com/jgarff/rpi_ws281x/blob/52aff5b6dd569c82aade27f54a54508d73390eee/ws2811.c#L894-L904
why do you remove the package-lock.json file here? Is there anything we can gain from not having it? Personally I like to keep it in my repositories.. (I never really thought about the implications either way when it comes to something like this module).
I was to quick with pull request, to be honest I am still finding my way around Git ;).
Not sure what I did with URLs and names, I was just experimenting trying to make it new npm package that can be easily installed, but I am new to that so ... yeah
Version check as you can see it's handled on init in C library, so probably good to just take it out completely.
I remove package-lock.json as it should be created with "npm install" so it's always generated based on current use.
Not sure if that's a good approach, but previously in other projects I had issues when package.json was conflicting with package-lock.json and had to be removed priori npm install
.
Regarding the whole repo, I am now not sure which one is better 1.x
or 0.10.x
I made both of them work with latest Node
Problem with 0.10
is that I had to change #define
variables in cc
file to match what I need, and also swap channel for 1
I like the minimum setup approach, and render approach from 0.10
but then I need to be able to create setup like in 1.x
I am now confused which one should I use. but I am assuming 1.x
is the latest one right ?
Yeah, I was planning on adandoning/replacing the 0.10-branch a while ago. The new version has much simpler bindings (less C++). But it's not tested much yet (at least that I know of...), so I hesitated.
finally got around to integrating your changes into the 1.0-branch, see here.
If possible I would be very happy to get some feedback if this works as intended, as I currently don't have a working setup around.
Thanks again for your patch!
Modified to work with latest Node 14