New Module: led_effect

[mental405]

The Neopixel and Dotstar support is really neat and I like the addition. I have been playing with addressable LEDs for a while on other platforms and have some nice effects written. While I can do many of them using jinja templates, it gets really really cumbersome with the template expansion and maintaining states. It also becomes problematic to maintain a decent enough framerate so the effects appear smooth along with keeping the "Transmit=1" from causing timing issues when updating too many things at once.

I started writing a module that extends the basic Neopixel and Dotstar classes to allow effects to be run independently of gcode commands. It works pretty well so far but I have a couple of questions for things I would like to do or I am doing.

Firstly, the mcu command for the neopixels expects a clock time for execution. The neopixel module as written does this

     print_time = self.printer.lookup_object('toolhead').get_last_move_time()
    self.send_data(self.mcu.print_time_to_clock(print_time)) 

in my module this works most of the time until you start trying to do a homing move, then the whole thing stalls because something breaks in the timing. To get around this, I just send everything with the default time of 0 since most of the time I don't care when it gets there.

I am not sure if this is resulting in the undesirable flashes of color making their way into the LEDs or if that is because of noise on the data line. They only start acting crazy when the Z motors are moving as the Neopixel strips are attached to that board with the Z motors. I need to test them on the other board controlling my XY E axes to see if it is replicated.

The module has several different effects classes that are subclassed out of a master effect. Each generates a "frame" for its effect when a NextFrame method is executed. The frames are generated out of a callback that is being executed from a master timer in the reactor that runs at the specified frame rate. So every n milliseconds, the next frames are generated and applied to the LED strips.

My only issue and question regarding this whole mess is with the time parameter on the MCU command, is it necessary to have the command tied to print time? Could that be the reason for my errant colors, especially if the data is getting split between multiple data packets?

Here is an example of an effect. You can see a green flash when the toolhead lifts. https://www.youtube.com/watch?v=hRC0B4cQJHw

[klipper-gitissuebot]

Hi @mental405,

It did not look like there was a Klipper log file attached to this ticket. The log file has been engineered to answer common questions the Klipper developers have about the software and its environment (software version, hardware type, configuration, event timing, and hundreds of other questions).

Unfortunately, too many people have opened tickets without providing the log. That consumes developer time; time that would be better spent enhancing the software. If this ticket references an event that has occurred while running the software then the Klipper log must be attached to this ticket. Otherwise, this ticket will be automatically closed in a few days.

For information on obtaining the Klipper log file see: https://github.com/KevinOConnor/klipper/blob/master/docs/Contact.md

The log can still be attached to this ticket - just add a comment and attach the log to that comment.

Best regards, ~ Your friendly GitIssueBot

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

[KevinOConnor]

It sounds like there are a few separate items here: a question on new modules, a question about timing, and a bug report on SET_LED.

As to adding a new module - it sounds like a useful addition. It's hard to say without seeing the code.

The timing is there so that SET_LED updates appear in order with the g-code stream. Otherwise, an led update could appear to occur before it was sent (for example, a "G1 F1 X200 ; SET_LED ..." could end up setting the led prior to the move even starting). The timing gets translated to a send_cmd(..., minclock=X) which ultimately enforces a minimum transmit time on the command messages sent from host to micro-controller.

As to incorrect colors - we really need to see the Klipper log file to comment. I'd guess you are seeing this with the neopixel (and not the dotstart). I'd further guess that the issue is due to the hard-coded timing in src/neopixel.c. Unfortunately, there are a wide range of neopixel devices out there, they have really bizarre timing restrictions, and to date no one has cared enough to harden the timing so infrequent errors like the above are avoided.

-Kevin

[mental405]

Thanks for the response @KevinOConnor

They are GRB WS2812B LEDs. I would like to say they are dotstar but those things are expensive. I have noticed most of the "artifacts" are green which to me sounds like there is extra "HIGH" signal when there is supposed to be a blank time in between color bytes. I will try to commit the module to my Git repo later with the complete source code but it is ugly and full of half written methods and placeholders.

My bigger question was related to sending an actual time code with the MCU command vs just letting it land whenever. When I send a timestamp like you are doing here

        print_time = self.printer.lookup_object('toolhead').get_last_move_time()
        self.send_data(self.mcu.print_time_to_clock(print_time))

it causes an MCU error during homing when it is polling the endstops. Since it is being set on the lowest priority MCU schedule, it is my understanding that the timing may not be respected anyways. (I am still learning a lot of this stuff). So if the timing only exists to ensure that it stays in sync with the GCODE, i think for this application it is not really needed.

This is a very abbreviated sample with the relevant bits included

import neopixel, dotstar

class ledEffect:
    def __init__(self, config):
         #read in config file, set printer and local variables.... basic module stuff

    def _handle_ready(self):
        self.reactor = self.printer.get_reactor()
        self.leds = self.printer.lookup_object(self.ledName.replace(":"," ")) 
        self.frameTimer = self.reactor.register_timer(self._frameCallback, self.reactor.NEVER)     

    def cmd_SET_LED_EFFECT(self, params):
        self.reactor.update_timer(self.frameTimer, self.reactor.NOW)

    def _frameCallback(self, eventtime):
        self.leds.color_data = self.nextFrame(eventtime)
        self.leds.send_data()

        #update timer for next frame at desired time        
        return eventtime + self.frameRate

I am still troubleshooting the random colors thing, I think it has to do with the actual timings of the chips on the strip like you said since they are rather wonky. I will try to get some logging data later.

Another question. Is there any way to keep a timer executing on the MCU during a shutdown state? For example. If there is a thermal runaway or something that causes everything to halt and catch fire, could commands still be sent to the MCU that cause the LEDs to blink red, for instance, to signal an error state? The timer and callbacks still run in the reactor, and the MCU is still transmitting serial data, but predictably the LEDs don't do anything.

[KevinOConnor]

it causes an MCU error during homing when it is polling the endstops.

There's little I can help without the klipper log file.

FYI, I updated the code (commit 0ae20421) so that the critical timing can be set in the host code (instead of in the micro-controller).

Is there any way to keep a timer executing on the MCU during a shutdown state?

It's possible to allow certain mcu commands to run even if the micro-controller is shutdown (see HF_IN_SHUTDOWN flag). It's also possible for the micro-controller code to reschedule a timer from its shutdown handler (as the adccmds.c code does). These types of actions should be done with caution though - as we don't want to make a bad situation worse (eg, if the hypothetical fire was in the led strip itself...).

-Kevin

[mental405]

I appreciate you adding the timings. This will give me something to play with.

I agree with certain situations adding additional risk. I think with LED's the risk both minimal and uncontrollable with firmware. Since the VCC and GND lines have constant supply as well as the chips themeselves. Outside of running them through an electronically switchable circuit with some sort of smoke detection... /shrug.

If I am understanding correctly I could create a command registration in neopixel.c

//Call this when the effect is created
void command_neopixel_configure_shutdown(uint32_t *args)
{
   //set up blinkenredlight 
}
DECL_COMMAND(command_neopixel_configure_shutdown, "neopixel_shutdown oid=%c data=%*s");

//do this when the MCU shuts down
void neopixel_shutdown(void)
{
     shed_add_timer(blinkenredlighttimer)
}
DECL_SHUTDOWN(neopixel_shutdown);

or

change the command registration macro for _command_neopixelsend to:

DECL_COMMAND_FLAGS(command_neopixel_send, HF_IN_SHUTDOWN, "neopixel_send oid=%c data=%*s");

[KevinOConnor]

FYI, one should be able to use the following test procedure to find the ideal neopixel timings for a particular strip:

Create a gcode file with the following:

SET_LED LED=my_led
G4 P1000
SET_LED LED=my_led RED=.01
SET_LED LED=my_led BLUE=.01

Then upload that gcode file, make sure the printer is idle (eg, no heaters running, no fans running), and run that gcode file. One should observe the LED strip showing red or blue. If the value of RESET_MIN_TIME in klippy/extras/neopixel.py is low, one may see the led turn all red, while if the value is large it will show all blue. At a mid point, one is likely to randomly see red or blue on each run. One can then modify the RESET_MIN_TIME setting in neopixel.py, run sudo service klipper restart, and print the file again to see if it reliably results in red or blue.

One can use this procedure to find the maximum value that results in all red (max_red_time) and the minimum value that results in all blue (min_blue_time). The ideal RESET_MIN_TIME is min_blue_time + 0.000010 and the ideal BIT_MAX_TIME is max(max_red_time - .000010, max_red_time / 2.0, .000004).

-Kevin

[mental405]

Initial testing after upping the blank time to 60 microseconds is positive. There are no longer flashes of errant color sneaking in that I have noticed.

Two Questions:

1: I am a little confused on OID's, can I and should I reuse the OID for the another command if that command has the same parameters and is no longer being used or should I generate a new one?

I added the HF_IN_SHUTDOWN flag to the neopixel_send mcu command and it is working well. Everything shuts down the way it should and the lights do their thing as you can see from my simulated thermistor failure video below. I would almost prefer to have a separate command for just for the shutdown effects so that any other running effects will be ignored in the case something doesn't exit cleanly in the host when a shutdown event is raised. Incidentally I am handling the shutdown even to un-schedule the reactor timers and schedule the shutdown timer.

2: There is a hard coded limit of 18 emitters. Is this number arbitrary or is it the max number of bytes that can be encoded into a single MCU command with the rest of the command overhead?

https://www.youtube.com/watch?v=8sOzOhYEFsI

[KevinOConnor]

I am a little confused on OID's, can I and should I reuse the OID for the another command if that command has the same parameters and is no longer being used or should I generate a new one?

One OID (Object identifier) is created for each config_something mcu command. There is a little info on this at: https://www.klipper3d.org/MCU_Commands.html#low-level-micro-controller-configuration . In short, the oid identifies which stepper, which pin, which neopixel, etc. that a command should affect.

I would almost prefer to have a separate command for just for the shutdown effects so that any other running effects will be ignored in the case something doesn't exit cleanly in the host when a shutdown event is raised.

I think you should be able to add a DECL_COMMAND_FLAGS(command_neopixel_send, HF_IN_SHUTDOWN, "neopixel_emergency_send oid=%c data=%*s"); declaration to have two commands.

There is a hard coded limit of 18 emitters. Is this number arbitrary or is it the max number of bytes that can be encoded into a single MCU command with the rest of the command overhead?

The latter.

-Kevin

[KevinOConnor]

Any further updates on this?

-Kevin

[mental405]

The timing issue has been resolved since I can now adjust the pulse duration. I parameterized it in the config since there is the possibility that people would have different strips with slightly different timings.

I have been too busy to work on adding additional features but I have enough of an idea of where to go that I don't think this issue needs to remain open. I was hoping some people would chime it with "You know it would be cool if...." and give me some suggestions for features. Oh well..