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

Quick fix to v3.0 #14

Closed stefancarlton closed 9 years ago

stefancarlton commented 9 years ago

Hi,

I've been working with your v3.0 code (from PR #13) and had an error where .render expected a Uint32Array - by recasting it like this code the error is resolved, however I'm not entirely sure if this is the right fix (I just needed it working to test everything).

Thanks for the code!

usefulthink commented 9 years ago

Hi Stefan,

first of all, thanks a lot for your PR, I appreciate it a lot!

This issue has bugged me for a while, and I still have a bit of trouble explaining what is going on there, but a part of the problem is that the error-message was misleading and the c++-code for checking the argument-type was not accurate enough.

There has been a bit of discussion about this in #6 and i just merged a fix that resolves the problem on the native side of the module.

(the main problem was that typed-arrays with just a few elements store the data internally instead of in the external array that is expected. When you create a new Uint32Array-instance from the existing array this is resolved as both arrays need to share the same memory-segment and thus trigger the externalization, but i found a way to trigger this on the c++-side which doesn't have the overhead of creating a new Uint32Array with every render-call).

The new version is up on npm (v0.4.0) and it would be great if you could test it in your setup, I tested against node-0.12.1 and iojs-1.6.3 and it looks as if the problem is resolved now.

stefancarlton commented 9 years ago

Cool beans. Thanks for the explanation. I'll pull your fix and test it here later on today :)