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

Add support for dynamic ColorFeature selection rather than static compiled into the type like today #804

Open
Makuna opened this issue May 3, 2024 · 5 comments
Labels
enhancement investigating Currently under investigation for more understanding of the problem.

Comments

@Makuna
Copy link
Owner

Makuna commented May 3, 2024

Is your feature request related to a problem? Please describe.
Today, consumers that expose user selectable LED types require scaffolding to instance different classes of NeoPixelBus due to the ColorFeature selection being a template argument.

Describe the solution you'd like
A dynamic ColorFeature that is given an argument that defines the selection so that one NeoPixelBus type can be used with any LED types.

Additional context
Note this is not covering the similiar issue with the Method template argument, this is just for the Feature template argument.

@Makuna
Copy link
Owner Author

Makuna commented May 3, 2024

@blazoncek Per discussion, I am capturing your request into this issue, and I will capture investigations here. I hope you can follow along and provide feedback as you consider how this could be integrated into WLED.

Current investigation shows the following are currently supported and if exposed as an enum would be similar to...

enum ColorFormat
{
    ColorFormat_555_16bit, // 5 bits per element
    ColorFormat_777_24bit, // 7 bits per element
    ColorFormat_X8bit, // 8 bits per element
    ColorFormat_X16bit, // 16 bits per element

    ColorFormat_x8Bit_5BitStatic, // 8 bits per element plus 5 bit global brightness at full
    ColorFormat_x16Bit_x5BitStatic, // 16 bits plus 5 bit brightness at full per element

    ColorFormat_x8Bit_5Bit, // 8 bits per element plus 5 bit global brightness
    ColorFormat_x16Bit_x5Bit, // 16 bits plus 5 bit brightness per element

    ColorFormat_Sm16825,
    ColorFormat_Sm16803pb,
    ColorFormat_Sm16823e,
    ColorFormat_Sm16804eb,
    ColorFormat_Sm16824e,
    ColorFormat_Tlc59711,
    ColorFormat_P9813
};

@Makuna Makuna added the investigating Currently under investigation for more understanding of the problem. label May 3, 2024
@blazoncek
Copy link

Sure will! Thank you for support.
We are currently using:

#define B_8266_U0_NEO_3 NeoPixelBusLg<NeoGrbFeature, NeoEsp8266Uart0Ws2813Method, NeoGammaNullMethod>
#define B_8266_U0_NEO_4 NeoPixelBusLg<NeoGrbwFeature, NeoEsp8266Uart0Ws2813Method, NeoGammaNullMethod>
#define B_8266_U0_TM1_4 NeoPixelBusLg<NeoWrgbTm1814Feature, NeoEsp8266Uart0Tm1814Method, NeoGammaNullMethod>
#define B_8266_U0_TM2_3 NeoPixelBusLg<NeoBrgFeature, NeoEsp8266Uart0Tm1829Method, NeoGammaNullMethod>
#define B_8266_U0_UCS_3 NeoPixelBusLg<NeoRgbUcs8903Feature, NeoEsp8266Uart0Ws2813Method, NeoGammaNullMethod>
#define B_8266_U0_UCS_4 NeoPixelBusLg<NeoRgbwUcs8904Feature, NeoEsp8266Uart0Ws2813Method, NeoGammaNullMethod>
#define B_8266_U0_APA106_3 NeoPixelBusLg<NeoRbgFeature, NeoEsp8266Uart0Apa106Method, NeoGammaNullMethod>
#define B_8266_U0_FW6_5 NeoPixelBusLg<NeoGrbcwxFeature, NeoEsp8266Uart0Ws2813Method, NeoGammaNullMethod>
#define B_8266_U0_2805_5 NeoPixelBusLg<NeoGrbwwFeature, NeoEsp8266Uart0Ws2805Method, NeoGammaNullMethod>
#define B_8266_U0_TM1914_3 NeoPixelBusLg<NeoRgbTm1914Feature, NeoEsp8266Uart0Tm1914Method, NeoGammaNullMethod>
#define B_HS_DOT_3 NeoPixelBusLg<DotStarBgrFeature, DotStarSpiHzMethod, NeoGammaNullMethod>
#define B_HS_LPD_3 NeoPixelBusLg<Lpd8806GrbFeature, Lpd8806SpiHzMethod, NeoGammaNullMethod>
#define B_HS_LPO_3 NeoPixelBusLg<Lpd6803GrbFeature, Lpd6803SpiHzMethod, NeoGammaNullMethod>
#define B_HS_WS1_3 NeoPixelBusLg<NeoRbgFeature, Ws2801SpiHzMethod, NeoGammaNullMethod>
#define B_HS_P98_3 NeoPixelBusLg<P9813BgrFeature, P9813SpiHzMethod, NeoGammaNullMethod>

Edited to only have distinct of features included.

Some are named features but could be replaced by generic features if needed.

We are using uint32_t as a base color object (8-bit per channel, 0xWWRRGGBB). This is converted to a NeoPixelBus feature on-the-fly in each SetPixelColor() call. We do our own color order management to avoid calling too many templates.
For 16-bit depth/48-bit (or 64-bit) features the values are expanded using explicit NPB feature cast, like SetPixelColor(pix, Rgb48Color(RgbColor(col))).

@Makuna
Copy link
Owner Author

Makuna commented May 4, 2024

This coming refactor for this request will include dynamic color order with it, along with dynamic number of elements.

The constructor for NeoPixelBus will look similar to the following (details still being worked out, but in that test branch it already has BusView implemented. These changes are to facilitate a single class that is dynamically set when constructed rather than at compile time.

NeoPixelBus(const BusView& busView, 
	const BusPins& busPins,
	const ColorFeatureConfig& featureConfig) 

BusView is a class that will abstract the layout, today it is just count and the loop of smaller collection across a large strip (memory saving). Longer term it's the abstraction for matrix and possible multiple strip merging (lower priority for a while).
BusPins is a class that will abstract single wire (data) versus two wire (data and clock). Again, future it could manage mulitple pins.
ColorFeatureConfig is a class that includes the ColorFormat enum, the count of elements and their order.

struct ColorFeatureConfig
{
   ColorFeatureConfig(ColorFormat format, 
	uint8_t* order,
	uint8_t countOrder) :
	Format(format),
	Order(order),
	CountOrder(countOrder)
    {
    }

    const ColorFormat Format;
    const uint8_t* Order;
    const uint8_t CountOrder;
}

A collection of ColorFeatureConfig will be provided that maps to specific chips. These can be passed as that argument and would be defined like...

const uint8_t ColorOrder_Rgb[] = {ColorIndexR, ColorIndexG, ColorIndexB};
const uint8_t ColorOrder_Grb[] = {ColorIndexG, ColorIndexR, ColorIndexB};
const uint8_t ColorOrder_Rgbw[] = {ColorIndexR, ColorIndexG, ColorIndexB, ColorIndexW};
const uint8_t ColorOrder_Rgbwxx[] = {ColorIndexR, ColorIndexG, ColorIndexB, ColorIndexW, ColorIndexIgnore, ColorIndexIgnore};

const ColorFeatureConfig Ws2812RgbFeatureConfig( ColorFormat_X8bit, ColorOrder_Rgb, countof(ColorOrder_Rgb) );
const ColorFeatureConfig Ws2812GrbFeatureConfig(ColorFormat_X8bit,  ColorOrder_Grb, countof(ColorOrder_Grb) );
const ColorFeatureConfig Ws2816FeatureConfig(ColorFormat_X16bit,  ColorOrder_Grb, countof(ColorOrder_Grb) );

So, if there is a config you need, you can provide your own.
There would also be no reason to use these, as you could pass the individual arguments also,

new NeoPixelBus(PixelCount, {dataPin, clockPin}, { colorFormat, colorOrder, colorOrderCount } );

All examples above are a still a work in progress and some bits omitted (methods) to keep it simple to read.

@blazoncek
Copy link

blazoncek commented Jun 22, 2024

Thinking of it, we (in general, not just WLED) need:

a) number of channels (i.e. W, WC, RGB, RGBWC, etc)
b) channel order (not WLED but the driver does)
c) channel width (i.e. 4 bit, 5 bit, 8 bit, 16 bit, etc)

Looking at your feature set you’ve split those across multiple differently named features. If you want to simplify and only have one NeoFeature<Channels,Order,Width> you could easily do with a bit more involved processing in applyPixelColor() but that would incur speed penalty. Every abstraction does it.

copied from Discord for posterity

Forgive me, but my brain sometimes need to reiterate to absorb what was said.

@turbofex
Copy link

I design little LED controllers with OLED screens built-in, they're controlled via Wi-Fi network, it's wonderful if I was able to change the strip type and pixel count directly on the screen of the device, since DIY projects offen comes with a random idea and some LED strips lying around with random length/ chip type. please do update the dynamic change of T_FEATURE and T_METHOD if possiable , dynamic pixel count is already available and works like a charm,please do update the feature and method. sorry for my poor english, not a native speaker, and have a nice day!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement investigating Currently under investigation for more understanding of the problem.
Projects
None yet
Development

No branches or pull requests

3 participants