Support NeoPixelBus for LED Strip components

[frzme]

NeoPixelBus (https://github.com/Makuna/NeoPixelBus) is a library that allows controlling of various led strips, similar to fastled. NeoPixelBus supports RGBW light strips which fastled does not and works reportedly better on esp8266 when WIFI is also used (see FastLED/FastLED#306) - I have not personally verified that though.

As I have RGBW lightstrips I was very interested in the RGBW support and therefore created a light output component which is strongly inspired by the fastled light output component.
I'm not very good with C++ and it likely shows at some place or the other.

NeoPixelBus relies strongly on templates and does not implement a non templated interface, therefore NeoPixelBusLightOutputComponent is also templated which is a little awkward.

PartitionedLightOutputComponent allows creation of "light partitions" which control parts of the strip of a NeoPixelBusLightOutputComponent. A better way to implement this might have been to make a PartitionedLightState.
BaseNeoPixelBusLightEffect, similarly to BaseFastLEDLightEffect allows implementing effects which directly animate the NeoPixelBus, the mechanism implemented here reuses the NeoPixelBusAnimator which comes with the library.

Let me know what you think.

[OttoWinter]

So I'm not 100% sure this is how NeoPixel RGBW lights should be implemented. I mean the NeoPixelBus library is very nice and all, but I kind of don't like that users would need to learn two different ways of doing the same thing. Maintenance-wise it would also be kind of a nightmare.

I really like the fastled project and the library is extremely advanced from a C++ and microcontroller perspective. For example, the use of templates, structs and inheritance is just extremely well done with fastled, plus if you look at the ESP32 implementation it does some really advanced stuff with the RMT peripheral to offload the communication part to an external peripheral. In one of the RGBW issues in the FastLED repo a developer even said that most of their resources are going to RGBW development right now and that they expect a release soon. So if that holds true I would rather go with fastled because I've just had an awesome experience with using it.

That being said, of course Neopixel RGBW support in esphomelib is of course quite good as well and I would merge it into esphomelib when it's complete. I have some comments on your code (as per https://esphomelib.com/esphomeyaml/guides/contributing.html#contributing-to-esphomelib):

• Please split up your implementation from the header file (.h) into source files (.cpp) where possible. With templates this might be difficult sometimes, so you can leave those out for now.
• Method names are lower_snake_case
• The block-start curly braces { should be on the same line as the method definition/if statement, as per the google styleguide.

Also, have you considered writing an esphomeyaml component for this integration, it would allow many more users to use this.

[frzme]

I don't have any issues with fastled it's just that the fastled RGBW issue is rather old (August 2017 FastLED/FastLED#482 + FastLED/FastLED#579 ) and I didn't see much movement there. And when I researched how to use my sk6812 from esp8266 I got the feeling that people were saying that using Neopixelbus is the safer choice (because of Wifi interfering with strip timing when using fastled, I did not verify this)
I fully agree with your points about maintenance and the trouble associated having two components doing the same thing. I also considered making an abstraction inside of esphomelib for fastled/neopixelbus but I'm not sure if it's worth the effort. It would enable sharing of animations between the two libraries and could potentially make it easier in the future to get more native led strip support (ideally homeassistant should have a led strip component which esphomelib could implement, hass could then offer cool features like defining effects there or partitioning the strip in hass as opposed to on the controller - unfortunatly I feel like making that is beyond my available time)

I'll have a look at your comments and adapt the coding style where appropriate.

When the esphomelib parts are merged I'll have a look at making a esphomeyaml component when I find the time. Before I had a closer look I had originally hoped that support for RGBW led strips would already be there in esphomeyaml therefore adding it in should be the most logical thing to do.

[frzme]

I hope to have implemented your comments. I was indeed not able to move any template code to .cpp files (I tried and failed miserably even though this answer https://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file made me think it could work)

I also added a ".clang-format" file which configures the code style of the project according to what you defined (see https://clang.llvm.org/docs/ClangFormat.html). Unfortunately it's not perfect therefore it's not a good idea to just blindly reformat the existing code with it,

[frzme]

Hey, it's been a while, what are your thoughts on merging this? @OttoWinter

[OttoWinter]

Sorry for not taking a look at this PR again

I've looked into the neopixelbus library a bit more now, and it is quite a clean library. I've not used it yet but I agree it would be a valuable addition.

Before this can be merged, I would however like to see a couple of things:

• Documentation in esphomedocs
• Please add a file in the api/light/ folder so that sphinx can auto-generate the docs for it.
• Is the partioned light functionality really necessary? a) I personally don't really know what you would use it for (and I've not seen anybody request it for the fastled component yet) and b) if it's added, it should handle the whole light strip as partitioned. If I understand correctly, the current code makes effects act on the whole led strip, not only a subset of LEDs. I personally would say to remove it for now, and if there's many users requesting it we can still add it later.
• The future of esphomelib is the esphomeyaml framework, and I would like to keep the C++ and YAML versions as close as possible feature-wise. I can try to do the esphomeyaml side of things if necessary
  • The way make_neo_pixel_bus_light works in this code will make it difficult to implement in esphomeyaml, as there are no global variables there. I have an idea how to make sure this integration will be able to be implemented in esphomeyaml, can I write to your branch?
• The source code needs to be split up from the definitions. For example in Application:: make_neo_pixel_bus_light , if necessary, I can do that for you by writing to your feature branch.

[frzme]

I can remove partioned light for now, the implementation feels rather hacky. I'm using it on a large strip wrapped around my kitchen island. It allows me to control individual faces of the strip instead of the whole strip.
The interaction (or lack thereof) with animations is indeed not good.

Feel free to make changes regarding make_neo_pixel_bus_light, I couldn't easily find a solution without a global but again: I'm not good with C++ - I'm sure there is a solution.

I attempted to split all implementations/headers where I was able to do so. The interaction with templated classes/functions and implementations however is very hard to understand for me.

I'll have a look at your Code Review soon. Thanks for it. Feel free to make any changes you seem fit, I'll also have a look (tomorrow?)

[frzme]

I implemented most of your suggestions. Hope I didn't miss anything. I did not further split up implementations in ".h" files.
What do you mean by creating a file for sphinx in api/light/? Where exactly?
I can add documentation in esphomedocs, should I create a pull request there which you would merge after this one?

I would also like to see both the "white LEDs only" (that's a power saving feature) and "partitioned light" implemented somehow. For the partitioned light I feel that the cleanest implementation might be a "partitionable light state" (instead of a partitionable light output) but doing that felt like a rather high effort at the time. What do you think?
The goal with the partitions is to be able to control individual parts of the strip from HASS and have them act like individual lights. An alternative approach might be to teach HASS about light strips and have HASS partition them, but that's a different can of worms (and what about animations then?).

[thoscut]

I was hoping on the merge of the PartitionedLightOutput, as beside the use case of @frzme I have a one as well, trying to control a Neopixel ring segment wise. But I also see, that the concept is not trivial as splitting up the whole strip into single light could get a mess on the HA side with large strips. Latency could be another problem when talking about animations.
I could make a new issue for the partitioning as it's basically independent of the NeoPixelBus.

[olivierguerriat]

I'm also interested in both Neopixelbus support and partionned strip, but I agree they're separate issues (and I will gladly use neopixelbus without partitions in the meantime).

[OttoWinter]

Ok, so I spent some time looking more at how to best integrate this into esphomelib so that in the future these things can be done without any major additional core changes:

• Partitioned Lights
• esphomeyaml support for neopixel busses (namely storing the controller inside the output component & not using as much templates)
• having a unified effects API for both fastled/neopixelbus. Because for effects, I wouldn't want to have to maintain two versions for each effect.

So, I implemented the concept of addressable lights in #243. They're a layer between the internal library (fastled/neopixelbus/...) implementation of addressable lights and esphomelib.

The advantage of that API is:

• It will work across libraries.
• It doesn't use any templates which would make implementing it in esphomeyaml quite a mess.
• White value is supported too. The ESPColor struct always has a white value even if the light strip doesn't support it. But that's not really a problem since the struct would be 32 bits long anyway.
• I would say it has a nicer API than neopixelbus (operator overloading, see sample light effects)
• It also supports color correction - so gamma correction / max brightness per channel will work independent of the capabilities of the internal library.
• You can still use the internal library (for example neopixelbus) if you like. See get_controller

It'll need quite a bit of testing/documentation, but I think I'll do that when I am finished with my 1.9.0 beta fixes.

[OttoWinter]

Closing for #352