Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thoughts about wasting 1 byte on universe update #39

Closed
Fensterbank opened this issue Jan 11, 2018 · 2 comments
Closed

Thoughts about wasting 1 byte on universe update #39

Fensterbank opened this issue Jan 11, 2018 · 2 comments

Comments

@Fensterbank
Copy link
Member

I'm not sure what to think about this change: 00d960e

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
Copy link
Member

wiedi commented Feb 19, 2018

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants