sync channel for fast move_queue population

[Cirromulus]

This PR adds the possibility to update SW/HW PWM faster than the original implementation (0.100s).
This would close #133, enabling laser raster engraving.
While it will probably not reach the stepper step-times (which are compressed), I suppose it may come close to some multiples of cycle_time in bursts, depending on the serial bandwidth to the MCU.
Early testers report around 400-800 updates per second, peaks higher (as the queue is buffered).

Known Issues

• Code seems to work on some different hardware setups. More testers appreciated!

If you want to try this out, you may follow this small HowTo.

How it works

This PR adds a "sync_channel" (name may be subject to change) to the steppersync/stepcompress system.
Basically, it allows a command to be inserted into the mcu's move_queue while respecting the watermark and timing.
This enables much tighter packing of pin-update commands.
To ensure the steppersync queues are flushed regularly even when the toolhead is not generating any movement,
a combination of note_kinematic_activity(), update_move_time() and check_stall() is used.

Other Notes

PWM Frequency: I suggest to not mean too well with the tool frequency. Choose the lowest frequency that still has a pulse time of a small multiple of your highest resolution. E.g. Resolution of 0.1mm at a speed of 50mm/s I recommend 1kHz (cycle_time: 0.001)

Hardware-PWM may be less calculation intense on the MCU, but the MCU will choose in "best effort" a frequency that is near the desired frequency.
The maximum pin update-rate is this chosen frequency.
Software-PWM will be less precise, but may offer more flexibility at odd frequencies.
So if you plan to have an update rate of 1000 updates / s, choose at least a frequency of 1kHz, thus a cycle_time of 0.001.

Unaddressed features, Roadmap

1. This PR wraps the "ht-mode" (high throughput) at the command wrapper, reducing the impact on MCU_pwm code.
   In the state of this PR, one pin can be put in ht-mode. In long term (another PR), I plan to rework the stepcompress system, dividing the two functionalities (1. compressing steps, 2. making sure the move queue is populated and ordered), so that the PWM updates don't need to "piggy back" on kinematics-stepper update flushes. This would enable even faster pinupdates and, most importantly, any number of ht-mode pins.

2. This PR only adds the possibility to update a pin much more frequently.
   For laser applications, a proportionality feature is useful. This would dim the laser during acceleration and deceleration phase proportional to its actual speed (see also S-Parameter in G0 in RepRap documentation), just like it does with the extruder E command.

[Arti4ever]

Hi @Cirromulus,
I am very interested in using laser engraver with Klipper, so I gave a try to your PR.
Unfortunately, your code do not work for me, Klipper crash when executing homing with error : " Exception in flush_handler".
Logs for this error : klippy_G28.log

I also tested just using M3 S5 command, but there is also an error : "MCU 'mcu' shutdown: Timer too close"
Logs for this error : klippy_M3.log

I hope this may help you to fix theses errors. Or maybe advise me if I am doing something wrong ?

[Cirromulus]

This may be wrong on non-8-bit boards

[Cirromulus]

Thanks for testing!
I will look into that soon.
You are using a 32bit board, do you?
I never tested it with one, so maybe the manual cap on the scheduled time is wrong (mcu.py)
Just for kicks, does it behave the same when using hardware_pwm: true ?

[Arti4ever]

Yes I am using a 32bits board.
Using hardware pwm do not work for me. It raise a config error.

[Arti4ever]

I finally tried with hardware PWM (support for LPC176x was added 2 weeks ago, so my mcu was not up to date, that's why it did not work at first...), but I have the same behavior.

[KevinOConnor]

Interesting. I agree this functionality would be good to add to Klipper.

At a high-level, the code doesn't look correct to me - at a minimum, the set_pwm() command has to be able to work from a background thread, which means it isn't valid for that function to call the toolhead code. (The heater code will change PWM settings directly from the background thread used to process temperature reports.) Alas, I haven't had time to look into this, so I don't have any implementation advice.

-Kevin

[Cirromulus]

Hi Kevin, thanks for your first feedback.
I will look into that (I believe I initially produced the same condition in the pin safety timeout PR).
So basically, it is not safe to call ....send([self._oid, clock, v], minclock=minclock, reqclock=clock) inside the GCode-code?
I suspect implementing a similar style as the command_queue would be more elegant and probably allow for concurrency.

[KevinOConnor]

So basically, it is not safe to call ....send([self._oid, clock, v], minclock=minclock, reqclock=clock)

I'm not sure about the above. My comments were referring to the self._th.note_synchronous_command(print_time) call from set_pwm() - it's not valid to call toolhead methods from a background thread.

-Kevin

[Cirromulus]

@Arti4ever If you want to try it again, it may be better now.
Toolhead code is now called by the reactor.

@TazerReloaded
What PWM frequency do you use? The Code limits a maximum update frequency to the pwm frequency.

[TazerReloaded]

I wrote this config:

[output_pin intensity]
pin: P2.0
pwm: true
cycle_time: 0.001
scale: 255
value: 0
shutdown_value: 0
[gcode_macro M3]
default_parameter_S: 255
gcode:
  SET_PIN PIN=intensity VALUE={S}
[gcode_macro M5]
gcode:
  SET_PIN PIN=intensity VALUE=0

So currently I'm using 1kHz software PWM i guess, which is a bit slow considering 50mm/s at a resolution of 0.1 already means 500 pin updates per second. How fast can/should I go with software PWM?
I also tried hardware_pwm: True, but that did not change the frequency (checked with scope), despite having the newest (as of yesterday) firmware built from your branch on the SKR. Do I have to specify a frequency for hardware PWM to work? Pin 2.0 seems to be PWM capable according to the chip manual.
I also noticed that my laser seems to respond to low analog voltages. I have a 470 Ohm resistor on the PWM line to protect the MCU from over current (already killed one with a short to VCC due to bad wiring), which means the pull-up on the laser board always pulls the PWM line to around 0.2V when the duty cycle is 0, and that makes the laser shimmer a bit. I use that like a feature because that low intensity can be used to align the laser with the zero point, and when not in use the 12V power to the laser is cut via a relay. I don't think it affects the PWM signal.

I also pulled your latest changes, will continue testing next weekend. Awesome work so far, already works great, thank you!

[Arti4ever]

It but it still doesn't work (for me at least) when I try do to a homing. the error is not the same though : " Failed to home x: Timeout during homing"
Same config on the master branch works great.
Here my log : klippy.log

[TazerReloaded]

Also noticed an issue while trying to print today: Whenever I enable a heater, Klipper shuts down with a "Timer too close" error after a few seconds. Same config on master works fine. Klippy log attached.
klippy.log

[Cirromulus]

How fast can/should I go with software PWM?

That depends on the computing power of your board. I suggest testing higher freqencies and measuring the jitter with a oscilloscope.

I also tried hardware_pwm: True, but that did not change the frequency

Then, the hardware-pwm was able to be configured to your desired frequency.
With cycle_time: 0.001 you specified a frequency of 1kHz (1/cycle_time). My board can't go faster, but yours might.

Whenever I enable a heater, Klipper shuts down with a "Timer too close" error after a few seconds.

I can reproduce that. I suspect that my way of toolhead.note_synchronous_command() is susceptible to concurrently flushing the move queues (in the past).
This is because I somehow rely on the move commands in the steppersync system to flush regurlarly.
This way, pwm-commands do not starve the stepper-queues. This introduces a lot of flushing and concurrency issues, however.
I will need some time to analyze the previously-used command_queues handled by the serial (slow) path.

[Cirromulus]

@Arti4ever I see that you use a BL-Touch as well. Does it even stow?
I had this problem during development as well, but it was solved within the last commits.

I like the fact that you also can cut the power supply to the Laser.
But it is possible, that this breaks the setup (two different pin-updates immediately after each other... did not test it yet!)
Klipper will try to update these pins exactly at the same time, I suppose.

[TazerReloaded]

The relay for laser power is attached to the Pi and controlled via Moonraker power commands, so I don't think that affects Klipper in any way. I also control the printers 12/24V PSU and the enclosure lights that way. I usually leave the room before powering the laser, don't want to take any risks, and if anything goes wrong, I can cut the power to everything remotely.
I can't think of an use case where pin changes have to happen exactly simultaneously, maybe if someone builds a needle printer running Klipper, but from an user perspective I'd expect it to be possible to control multiple pins without issues.

[TazerReloaded]

I tested the output frequency today, and on my hardware (SKR 1.4), I get a nice clean 10.8kHz square wave with the following settings:

hardware_pwm: true
cycle_time: 0.0001

It is a little bit faster than the expected frequency, but that could be caused by the hardware PWM clock scaling. Edges look very clean and even values like 0.005 (0.5%) are represented accurately.
I also installed a new laser, which responds very precisely to those PWM values. I'm cutting 4mm plywood, so no high speeds, but stock Klipper is still way too slow, leaving ugly marks when moving the laser before shutting it off. Your PR on the other hand works perfectly, no timing issues so far. If it wouldn't render heaters unusable at the moment I would already use it in my everyday firmware.

[Cirromulus]

Nice hearing that!
Unfortunately, getting the heaters to work requires partly a new concept.
Currently no free time for that milestone 🐱‍🐉

At least, one can check out the different branch, restart klipper and go on lasing.
Before pwm_queue was not implemented in the mcu firmware, one would have to re-flash the printer with every switch 🤕

[TazerReloaded]

Unfortunately, the time of re-flashing is coming back sooner or later. The master branch has already altered the communication protocol, and introduced a few fixes for my setup. I flashed a DFU loader onto my board, so at least I don't have to fiddle the memory card out of the electronics enclosure every time. According to GitHub here are no conflicts so far, so a patch would be possible, but I expect it to be only a matter of a few weeks until conflicts arise.
I'd like to help, but I'm also short on free time, and I doubt that I can come up with a better solution without investing at least a few days into understanding the inner mechanisms of Klipper.
Anyways, thank you for all the effort so far, it has some edges, but it's a working solution!

[Cirromulus]

According to GitHub here are no conflicts so far, so a patch would be possible, but I expect it to be only a matter of a few weeks until conflicts arise.

You are correct.

I doubt that I can come up with a better solution without investing at least a few days into understanding the inner mechanisms of Klipper.

That is my problem right now. I need a free weekend to uncouple the sync_channel from the move queues.
Right now, if any pin-update flushes the move queue (or the mq itself), a different pin (e.g. from a background task) will queue its update, but will not be flushed till the next round, which may or may not be too late for the update to happen.

[Alexdad76]

"Timer too close" error after a few seconds after the command M3 (1-255) . Same config on master works fine. Klippy log attached.
SKR 1.1
klippy.log

[Alexdad76]

According to GitHub here are no conflicts so far, so a patch would be possible, but I expect it to be only a matter of a few weeks until conflicts arise.

You are correct.

I doubt that I can come up with a better solution without investing at least a few days into understanding the inner mechanisms of Klipper.

That is my problem right now. I need a free weekend to uncouple the sync_channel from the move queues.
Right now, if any pin-update flushes the move queue (or the mq itself), a different pin (e.g. from a background task) will queue its update, but will not be flushed till the next round, which may or may not be too late for the update to happen.

I want to clarify. Does you need more information? #4128 (comment)

[Cirromulus]

Your log points to an error in the message flush system. This relates to the strong coupling between stepper moves and the pin output.

Line 870: Sent 99 5414.972272 5414.972272 14: seq: 10, queue_pwm_out oid=4 clock=1735086958 value=100
Line 971: Receive: 99 5414.973191 5414.972272 12: seq: 11, shutdown clock=1745260113 static_string_id=Timer too close

This indicates that the second queue_pwm_out was sent out far too late (10.173.155 clock cycles, or around 0,1 seconds).
Do you have maximum_mcu_duration turned on? If configured, I suggest either turning it off or increasing the value for testing purposes.

[Alexdad76]

maximum_mcu_duration in config turned on:
maximum_mcu_duration = 5

When trying to turn off an error occurs:
maximum_mcu_duration = 0
Error: Option 'maximum_mcu_duration' in section 'output_pin TOOL' must have minimum of 0.5

When trying to increase more than 5, an error occurs
maximum_mcu_duration = 15
Error: Option 'maximum_mcu_duration' in section 'output_pin TOOL' must have maximum of 5.0

[Cirromulus]

Yeah, this one is a bit tricky. To turn it off, you have to delete the line. Then it will default to zero.

[Alexdad76]

Yeah, this one is a bit tricky. To turn it off, you have to delete the line. Then it will default to zero.

HOORAY! Klipper stopped shutdown! The idle passage without a laser was successful. The LED on the output of the fan worked correctly.I will try real burning. Thank you so much for the hint!

[Cirromulus]

No Problem. Once this system gets more stable, you should activate it again, though. For safety ;)

[Alexdad76]

No Problem. Once this system gets more stable, you should activate it again, though. For safety ;)

Everything works fine:

Laser: NEJE Module 7W
Software: https://laserweb.yurl.ch/
Mode: "Laser Raster"
Laser Diameter: 0,1
Speed: 80 mm/s, Power: 0-80%

1. Heard strange knocks over each square.
2. There are some artifacts - lines about 6 and 7 - the laser was not turned off (maybe the G-code bug).

By the way, LaserWeb does not create a G-code if in the section Settings\GCODE"TOOL ON", use M3$Intensity. It works only simply M3

Update:
Strange, now the code is generated and with M3 $INTENSITY

Just in case I attach klippy.log
klippy80_80.log

[KevinOConnor]

FYI, I suspect the fixes in #5975 will improve the results of this code.

-Kevin

[Cirromulus]

Thanks, I will rebase and test it on my system. Perhaps it has something to do with this issue where a user gets not-flushed elements after a CANCEL_PRINT and a timer-too-close when it moves again.

[Cirromulus]

Rebased. Users report that this indeed fixes the CANCEL_PRINT issue.

[Rneil-ca]

Hi Cirromulus,

Just wanted to say thank you for creating this fork. I have it successfully working on a skr v1.4 board with a Chinese laser with a 5v pwm input. Just got it working tonight and so far it is working great. Found your fork when I wasn't able to get my laser to burn properly with the standard version of Klipper.

Thanks again for all the work you have put into this.

Ryan

[Rneil-ca]

Any updates on this being merged into the main Klipper branch? I have been using this for months and it works great.

Thanks,
ryan

[naikymen]

Same here! It does require a bit of merging work though.

I merged the branch yesterday, but some functions have changed names in upstream Klipper since this PR.

I don't understand that part of the code, so It's a bit beyond my capability to update it for now.

[Cirromulus]

@Rneil-ca nice to hear. Do you use it for both printing and lasing purposes?

@naikymen Do you need a specific feature of the klipper master branch? The fork works well for me, and I grow tired of the continuous rebasing.

[naikymen]

Hi @Cirromulus, thanks for the reply.

Yes but sort of... I need to use this fork, aimed at multipurpose CNC, which I keep in sync with upstream klipper.

I keep it synced because I like the fixes and new features across the complete stack (i.e. MR and Mainsail). I do not plan to open more PRs here, as they are a lot of work and hard to get merged (from my perspective).

I grow tired of the continuous rebasing.

Makes sense. I was still expecting that your work be merged to master, but if this is not the case then I'd humbly request one more rebase.

Some folks and I have been discussing klipper for more general CNC over here, and yours is the only branch implementing laser support that I'm aware of.

I'd gladly support the rebase with a small donation if any of this makes sense to you.

Thanks for your work!

[Rneil-ca]

@Rneil-ca nice to hear. Do you use it for both printing and lasing purposes?

This machine does both CNC and Laser, I use normal Klipper on my 3d printers. So far this has been working great, so no need anything now. Would be nice if this was permanently merged into the main branch so we can stay up to date, but not sure if they are willing to do that.

Thanks again for all your work on this!

Ryan

[Cirromulus]

Rebased against current master.

Some folks and I have been discussing klipper for more general CNC over here, and yours is the only branch implementing laser support that I'm aware of.

Also cool to hear. I followed the thread a bit. Hopefully someone contributes a 'XY-only' kinematic for the folks without Z axis.

I'd gladly support the rebase with a small donation if any of this makes sense to you.

Thank you very much, but I want to give back to the FOSS-community when I find the time :)

[Cirromulus]

@Rneil-ca

This machine does both CNC and Laser, I use normal Klipper on my 3d printers.

The thing @KevinOConnor is most concerned about is that nothing breaks normal 3d printing operations.
So you could help us with using the fork on your actual printers as well, and then report back whether you encounter any problems or not (my preferred feedback 😄).

[ssendev]

It's been some time but when I tested it printing worked fine but it crashed KlipperScreen which was the reason i didn't leave it installed permanently.

[naikymen]

Also cool to hear. I followed the thread a bit. Hopefully someone contributes a 'XY-only' kinematic for the folks without Z axis.

I think this is working already, there is a config draft over here: https://gitlab.com/pipettin-bot/forks/firmware/klipper-stack/-/blob/pipetting/printer_data/config/cnc_shield_xy/printer.cfg#L19

We're making a new machine so I haven't had much time to test these features.

Thanks a lot for rebasing, I've merged the branch without conflicts. 🥳

Over here: https://github.com/naikymen/klipper-for-cnc/tree/develop-pwm_sync_channel-merge

[Rneil-ca]

The thing @KevinOConnor is most concerned about is that nothing breaks normal 3d printing operations. So you could help us with using the fork on your actual printers as well, and then report back whether you encounter any problems or not (my preferred feedback 😄).

Thanks for the rebase, will install the latest on both of my printers and my CNC and will let you know if there are any issues.

Thanks again for all your work on this.

Ryan

[HWiese1980]

I've been running this change for about a year now on a Raspi Zero 2 and BTT Octopus without any issues. Printing works like a charm, as usual, just like using the laser engraving feature with a laser attached to a fan port. Never encountered any problems.

Please merge!

[KevinOConnor]

The functionality in this PR should now be on the main branch (with the merge of PRs #6369, #6410, #6420).

-Kevin

[github-actions]

This ticket is being closed because the underlying issue is now thought to be resolved.

Best regards,
~ Your friendly GitIssueBot

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