-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
sync channel for fast move_queue population #4128
Conversation
Hi @Cirromulus, I also tested just using M3 S5 command, but there is also an error : "MCU 'mcu' shutdown: Timer too close" I hope this may help you to fix theses errors. Or maybe advise me if I am doing something wrong ? |
klippy/mcu.py
Outdated
self._set_cmd.send([self._oid, clock, on_ticks], | ||
minclock=minclock, reqclock=clock) | ||
# queue_pwm_out oid=%c clock=%u value=%hu | ||
data = (self._set_cmd, self._oid, clock & 0xFFFFFFFF, v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be wrong on non-8-bit boards
Thanks for testing! |
Yes I am using a 32bits board. |
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. |
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 -Kevin |
Hi Kevin, thanks for your first feedback. |
I'm not sure about the above. My comments were referring to the -Kevin |
@Arti4ever If you want to try it again, it may be better now. @TazerReloaded |
I wrote this config:
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 pulled your latest changes, will continue testing next weekend. Awesome work so far, already works great, thank you! |
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" |
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. |
That depends on the computing power of your board. I suggest testing higher freqencies and measuring the jitter with a oscilloscope.
Then, the hardware-pwm was able to be configured to your desired frequency.
I can reproduce that. I suspect that my way of |
@Arti4ever I see that you use a BL-Touch as well. Does it even stow? I like the fact that you also can cut the power supply to the Laser. |
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 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:
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. |
Nice hearing that! At least, one can check out the different branch, restart klipper and go on lasing. |
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. |
1802c28
to
0fad198
Compare
You are correct.
That is my problem right now. I need a free weekend to uncouple the sync_channel from the move queues. |
"Timer too close" error after a few seconds after the command M3 (1-255) . Same config on master works fine. Klippy log attached. |
I want to clarify. Does you need more information? #4128 (comment) |
Your log points to an error in the message flush system. This relates to the strong coupling between stepper moves and the pin output.
This indicates that the second |
maximum_mcu_duration in config turned on: When trying to turn off an error occurs: When trying to increase more than 5, an error occurs |
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! |
No Problem. Once this system gets more stable, you should activate it again, though. For safety ;) |
Everything works fine:
By the way, LaserWeb does not create a G-code if in the section Settings\GCODE"TOOL ON", use Update: Just in case I attach klippy.log |
Hi, I could reproduce your problem; it had something to do with the wrong calculation of the guard time between beep and cycle_time change. |
2a709b3
to
b93ac33
Compare
FYI, I suspect the fixes in #5975 will improve the results of this code. -Kevin |
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 |
b93ac33
to
319e4e4
Compare
Rebased. Users report that this indeed fixes the |
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 |
Any updates on this being merged into the main Klipper branch? I have been using this for months and it works great. Thanks, |
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. |
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).
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! |
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 |
319e4e4
to
b93ac33
Compare
Signed-off-by: Pascal Pieper <[email protected]>
Signed-off-by: Pascal Pieper <[email protected]>
Signed-off-by: Pascal Pieper <[email protected]>
Signed-off-by: Pascal Pieper <[email protected]>
b93ac33
to
d55d303
Compare
Rebased against current
Also cool to hear. I followed the thread a bit. Hopefully someone contributes a 'XY-only' kinematic for the folks without Z axis.
Thank you very much, but I want to give back to the FOSS-community when I find the time :) |
The thing @KevinOConnor is most concerned about is that nothing breaks normal 3d printing operations. |
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. |
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 |
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 |
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! |
This ticket is being closed because the underlying issue is now thought to be resolved. Best regards, PS: I'm just an automated script, not a human being. |
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
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()
andcheck_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
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.
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.