electro-smith / libDaisy

Hardware Library for the Daisy Audio Platform
https://www.electro-smith.com/daisy
MIT License
312 stars 131 forks source link

Add driver for Adafruit Dotstar LEDs (SK9822) #574

Closed ndonald2 closed 1 year ago

ndonald2 commented 1 year ago

Summary

Adds device support for SK9822 addressable RGB LEDs (aka Adafruit Dotstar) with SPI Transport

Details

This is a device support "driver" class for the SK9822 RGB addressable LED. The code patterns are modeled after other device classes in libDaisy, implemented as a template class with data transport broken out separately. Currently there is only an SPI transport implementation but in theory one could add transport support for other peripherals.

The device class is meant to simultaneously manage the color states of 1 or more serially connected LEDs or "pixels" (currently up to a hard-coded maximum of 64, but this could probably be made a template parameter). The nature of the device is such that all LEDs must be updated at once, since the serial data must be clocked through the entire chain, so it's simplest to just keep all the data in memory until ready to display.

The usage should be fairly self explanatory based on the doc comments but an example is provided below.

Possible Future Improvements

The SPI transport currently uses blocking transmits rather than DMA. In practice this is probably OK for the typically small number of LEDs in synth designs and the fast clock speed in use (default baud prescaler of 4 which results in a clock rate of 6.25MHz seems to work fine for me). But it might be nice to have the option to do DMA based SPI writes instead.

Additionally, the Color class in libDaisy could be improved to handle many common tasks for working with Color data (in the context of addressable LEDs or otherwise):

Example Usage

For patch submodule, which uses SPI2 instead of the default SPI1

#include "daisy_patch_sm.h"

using namespace daisy;
using namespace daisy::patch_sm;

const uint8_t kNumLeds = 4;

static DaisyPatchSM hw;
static DotStarSpi dotstars;

int main(void)
{
    hw.Init();
    hw.StartLog();

    DotStarSpi::Config dotstar_cfg;
    dotstar_cfg.Defaults();
    dotstar_cfg.transport_config.periph   = SpiHandle::Config::Peripheral::SPI_2;
    dotstar_cfg.transport_config.clk_pin  = Pin(PORTD, 3);
    dotstar_cfg.transport_config.data_pin = Pin(PORTC, 3);
    dotstar_cfg.color_order               = DotStarSpi::Config::ColorOrder::RBG; // If colors seem off, change this
    dotstar_cfg.num_pixels                = kNumLeds;

    dotstars.Init(dotstar_cfg);
    dotstars.SetAllGlobalBrightness(1);

    float b = 0.0f;
    uint32_t val = 0;
    while(1)
    {
                // cube of ch brightness for gamma correction
        val = b * b * b * 255;
        dotstars.SetPixelColor(0, val, 0, 0);  // red
        dotstars.SetPixelColor(1, 0, val, 0); // green
        dotstars.SetPixelColor(2, 0, 0, val); // blue
        dotstars.SetPixelColor(3, val, 0, val); // pink
        dotstars.Show();
        b += 0.01f;
        if (b >= 1.0f) b = 0.0f;
        System::Delay(33);
    }
}
stephenhensley commented 1 year ago

@ndonald2 As always, thanks for the fantastic contribution!

This looks good to me! I don't have anything to test locally, but assuming it drives dotstars okay, I think this is probably ready to merge once you're satisfied.

Re. color improvements, I agree with what you suggest. Semi related, I actually think I might have a branch lying around somewhere that has some nice QoL stuff like basic constructors, and some operator overloads for adding, scaling, and blending. I think for utilities (esp. non-audio-rate) convenience can definitely outweigh convention. So I'm definitely open to that.

I'll see if I can find that sometime this week, and I'll make a PR.

ndonald2 commented 1 year ago

@stephenhensley I've been using this code on Warp Core for months and it's been working great, so I'm pretty satisfied. I know at least one other person on the Discord was using DotStars so I could reach out and ask them if they might be able to chime in on the PR if we want another set of eyes.

Semi related, I actually think I might have a branch lying around somewhere that has some nice QoL stuff like basic constructors, and some operator overloads for adding, scaling, and blending.

Awesome, I'd love to take a look at that. I ended up just creating my own Color class that does pretty much exactly what you're describing, plus constexpradoption across a lot of the methods (including constructors) for zero program memory (in some cases) color constant definitions, etc, and it's been great.

stephenhensley commented 1 year ago

@ndonald2 re. having another set of eyes: Up to you. If you've been using it, then I'm sure its good enough to start with, and we can always apply a patch if someone catches something.

And super cool! I'll take a look around Friday or this weekend to see if I can find that code and post an initial PR. I don't think I went and did the constexpring, but that would be a nice addition for sure.

ndonald2 commented 1 year ago

@stephenhensley cool cool, I'm satisfied so feel free to merge this anytime. I haven't exercised every part of the interface but all the basic stuff is working great (set constant-current brightness, clear/fill, set individual pixel colors, draw...)

If there's anything I'm slightly torn about it's exposing the full 5-bit range of constant current brightness in the interface, since really only the first 5 levels or so are practical for most use cases due to power consumption and thermal concerns. I guess the warning comment is enough? But I don't want anyone to fry their circuits by trying to ramp it up really high.

(also sorry for the double comments, I have another GH account I use for one of my gigs and forget I'm logged into that one sometimes)

stephenhensley commented 1 year ago

@ndonald2 Cool, well I think its ready then! I'll hit merge.

If there's anything I'm slightly torn about it's exposing the full 5-bit range of constant current brightness in the interface, since really only the first 5 levels or so are practical for most use cases due to power consumption and thermal concerns. I guess the warning comment is enough? But I don't want anyone to fry their circuits by trying to ramp it up really high.

This is a good point, but as long as the default is sensible, I'd suspect a user would see the warning at the same time that they see the function for updating it. So I think it's fine.

Thanks again for the PR!