adafruit / Adafruit_Seesaw

Arduino library driver for seesaw multi-use chip
93 stars 64 forks source link

seesaw_neopixel::clear(): Fix write buffer size #43

Open unvirtual opened 3 years ago

unvirtual commented 3 years ago

Fix buffer size exceeding I2C limit of 32 bytes in writes to seesaw. Currently, the code ignores additional two register bytes being prepended, causing seesaw_neopixel::clear() to silently fail.

unvirtual commented 3 years ago

I've added further commits to this PR:

Altogether, according to my measurements, these changes can reduce bytes written to the seesaw by more than 50% when updating a larger number of consecutive LEDs, which directly translates to faster updates, since at least on Teensy4.0 update rates are limited by I2C bandwidth.

Caveats:

Please let me know what you think and if we should proceed to get these changes merged.

ladyada commented 3 years ago

not sure yet! im a little intregued by the bitmask class, can you explain why you made it?

unvirtual commented 3 years ago

To be honest I just needed a simple way to to disable certain indices in the range in setPixelRangeColors() from updating. This way it's possible to e.g. update only every other pixel in the range and keep the current color otherwise (btw would be great if we could save those bytes from being transferred at all by changing the seesaw API)

Something like std::bitset would be best for this, but the number of Leds controlled by one seesaw seems to be run-time defined and can be arbitrary large. So this was just a quick hack to get the desired data structure aligned with the above requirement.

I'll gladly change this to any other better suggestion from your side that doesn't require BitMask at all ;)