adafruit / Adafruit_DotStar

GNU General Public License v3.0
97 stars 58 forks source link

Add support for passing in non-default SPI #48

Closed caternuson closed 1 year ago

caternuson commented 2 years ago

This was intentionally left out of #47 just to keep that PR as simple as possible, since it was a major conversion. With that PR now in place, can look at adding the ability to pass in a non-default SPI instance.

There is a semi-related PR #20. But that PR is based on the older non-BusIO version of the library.

ladyada commented 2 years ago

im ok with non-default SPI (you can just toss it right into the busio initializer) but def do not want SERCOM management ick :)

digitalcircuit commented 2 years ago

If the fix in "show: Use BusIO SPI transactions to set clock freq" #49 is merged, supporting passing in a non-default SPI might also provide a way to specify something other than 8 MHz SPI frequency, too - useful for long or short DotStar runs.

(Currently, the SPI clock frequency isn't being set correctly due to missing beginTransaction()/endTransaction() calls, so it can still be overridden the old way by doing a beginTransaction() outside the DotStar library before calling show().)

gigaj0ule commented 1 year ago

I’m interested in making this happen, what would be your recommended path?

Adding this seems to do it:

/*!
  @brief   DotStar constructor for hardware SPI. Must be connected to
           MOSI, SCK pins.
  @param   n  Number of DotStars in strand.
  @param   o  Pixel type -- one of the DOTSTAR_* constants defined in
              Adafruit_DotStar.h, for example DOTSTAR_BRG for DotStars
              expecting color bytes expressed in blue, red, green order
              per pixel. Default if unspecified is DOTSTAR_BRG.
  @return  Adafruit_DotStar object. Call the begin() function before use.
*/
Adafruit_DotStar::Adafruit_DotStar(uint16_t n, uint8_t o, SPIClass * mySPI)
    : numLEDs(n), brightness(0), pixels(NULL), rOffset(o & 3),
      gOffset((o >> 2) & 3), bOffset((o >> 4) & 3) {
  spi_dev = new Adafruit_SPIDevice(-1, 8000000, MSBFIRST, 0, mySPI);
  updateLength(n);
}
PaintYourDragon commented 1 year ago

Added in 1.2.2 (I’m really surprised this wasn’t already there…sorry!)

digitalcircuit commented 1 year ago

Thank you for adding this!

I did misunderstand how this would work - contrary to my past assumption in this issue, passing in a custom SPIClass does not appear provide a way to override the SPI frequency of 8 MHz (8000000 in Adafruit_SPIDevice constructor). This would help with long runs of DotStar LEDs (6 MHz is stable even with a voltage drop to 4 VDC, whereas 8 MHz requires closer to 4.5+ VDC).

Should I file a new, separate issue for a way to set the SPI frequency?