Skip to content

Conversation

@unvirtual
Copy link

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

I've added further commits to this PR:

  • clear(uint32_t): setsa color for all LEDs at once
  • setPixelRangeColors(): Update colors in a pixel range from an array of uint32_t (can wrap around to allow for simple repeating patterns). Minimizes bytes written to the seesaw.
  • setPixelColorsInRect(): Set MultiTrellis colors in a rectangle from an array of uint32_t
  • setRowColors(), setColumnColors(): MultiTrellis convenience functions to set single row and column colors

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:

  • BitMask class is very crude and not well tested
  • Interfaces are bulky/ugly
  • Not tested with more than 16 LEDs per seesaw (only tested with a matrix of 8 neotrellis boards)
  • Only tested on a Teensy4.0. Weaker platforms might not by limited by I2C performance at all.

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

@ladyada
Copy link
Member

ladyada commented Jun 3, 2021

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

@unvirtual
Copy link
Author

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 ;)

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

Successfully merging this pull request may close these issues.

2 participants