Add grouping for LEDs

[julianschill]

This adds the functionality to create groups of LEDs to either separate a chain or combine multiple chains to one big LED chain.

Configuration is like this:

[led_group mygroup]
leds:
  neopixel:grbw (2-4,6-7,10)
  neopixel:grb
  dotstar:my_dotstar (32-35)

The indexing is then done in the order of the group definition. The code was partly taken out from the LED effect module, created by Paul McGowan and maintained by me.

Signed-off-by: Julian Schill [email protected]

[KevinOConnor]

Thanks. Can you provide some background on why this feature is useful? Why would users want to organize LEDs into "virtual chains"? Can they use macros or display_templates for the same effect?

Separately, at a minimum, this code would need to be reworked to not "peek" into member variables of external classes.

-Kevin

[julianschill]

I can think of two setups where this is useful. First setup would be a really long chain, where e.g. the chamber LEDs are followed by a toolhead Logo LED and two nozzle LEDs. This is a common setup for Voron printers. So the users could define their virtual chains "chamber", "logo", "nozzle" and set them directly without memorizing the indeces. You could also assign display templates much easier to those LEDs.

Second application are several chains on different pins, that should be switched together. Like 4 chains on the top of the printer connected to four pins on the MCU. So you could switch them on and off with one command or apply a display template on all of them.

I think this could partly done by macros, but it would be much more convenient to have such an abstraction layer. This could then also be used by the frontends to switch those LEDs directly. So you could add a color selector for each LED group.

I will rework the code to use getter and setter functions.

[meteyou]

currently, I am also working on neopixel groups in mainsail. however, these are only "virtual" and therefore a single GUI solution. furthermore, I therefore also have to send a lot of gcode every time several neopixels are addressed. this would also look cleaner with the groups in klipper.

because I currently only have the stealthburner with neopixels (i personally don't use many light effects in my printers), I can only show it from this setup. mainsail always sends 2 lines for the nozzle LEDs. but if users have 10 neopixels per group, it will be a lot of gcode...

so I would very much welcome this function in klipper. just my 2 cents...

[julianschill]

I reworked the code and added a getter for the led helper. So it should not peeking into other classes anymore. Let me know if you see other code issues. Also because the led_helper is now looking up the name, the new format for the configuration is:

[led_group mygroup]
leds:
  grbw (2-4,6-7,10)
  grb
  my_dotstar (35-32)

So the type of the LED is not needed anymore. Therefore I added a check so all LED names are unique (I think that was missing in the current code).

[github-actions[bot]]

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[KevinOConnor]

Thanks. I noticed a couple of things looking through this PR:

1. An "extra" module should not store a reference to the "config" object. This just leads to confusion, as it's generally not valid to read config parameters after the initial printer setup phase. In some cases, it looks like this is just done for error reporting (where raise self.printer.config_error() is an easy replacement). I'm not sure about the other cases.
2. FWIW, the config format seems overly complicated to me. I fear adding in a new "syntax" will just be a burden to users.

Finally, I'm not sure I'm a good candidate to review this PR. I don't have any extensive LED setups and it's not a particular area of interest to me. Klipper needs more core developers willing to review PRs - without that, I fear this PR will not get sufficient review attention.

-Kevin

[github-actions[bot]]

Unfortunately a reviewer has not assigned themselves to this GitHub Pull Request and it is therefore being closed. It is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[Riconec]

sad it was closed. I have mini12864 with 3 neopixels and I wanted to split them into display backlight and encoder leds to maybe have effects on encoder but I don't sure I can do that in stock klipper

[jonferreira]

Shame this was dismissed. If you daisy chain LED Rings,for example,this is extremely useful!