node-dmx / dmx

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

Thoughts about wasting 1 byte on universe update #39

Closed Fensterbank closed 6 years ago

Fensterbank commented 6 years ago

I'm not sure what to think about this change: https://github.com/wiedi/node-dmx/commit/00d960e441024627e1023e2ac0776b899ed6b9af

Indeed this fixes the issue with the zero-indexed adresses, which is important because an API should not just wrap things 1:1 (which it did before), it should make things easy. And a dmx library is easy, when the channel shown on the DMX device can be used without illogical conversion.

But how the change is implemented, we loose 1 byte on every universe update, resulting in loosing performance I think. In most cases when updating some lights, the user don't send all 512 channels to the api. Only the channels which should change would be sent. So wouldn't it be a better idea, to process the given data in the library's update method (before sending it over the buffer) and substracting every key by 1? Indeed we would need to iterate through every key in the given state object and in worst case we would iterate through all 512 keys, but in most cases we would iterate only a few keys and avoid wasting 1 byte on every update.

What do you guys think about it?

wiedi commented 6 years ago

I think moving stuff around would cost more performance than skipping the one byte in front of the buffer. Aside from performance I'm more concerned about code complexity and off-by-one errors when mapping things everywhere.

With the current solution we just need to skip the single byte when writing the universe to the bus.

Also one byte per universe extra is pretty tiny and probably not something worth optimizing. But if this is something you want to work on I'm not going to stop you. ;-)

Fensterbank commented 6 years ago

Thanks for your reply. Makes sense for me and I'm still not sure, which solution would be best in terms of performance. So I think I'll close this for now.