Skip to content
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

M190 with Snnn and Rnnn parameters #1108

Closed
lmcbmai opened this issue Jan 13, 2019 · 15 comments
Closed

M190 with Snnn and Rnnn parameters #1108

lmcbmai opened this issue Jan 13, 2019 · 15 comments

Comments

@lmcbmai
Copy link

lmcbmai commented Jan 13, 2019

Hi,
I have a question about handling M190 command in Klipper. Here is M190 description from reprap.org wiki page:
m190
Here is how Marlin handles it:

  • if bed temperature is below the value that follows "S", printer waits until bed temperature reaches target value and then continues with the next command.
  • if bed temperature is above the value that follows "S", printer does not wait until the bed cools down to the target temperature and moves to the next command right away.

I discovered today that Klipper handles the 2nd scenario differently; it actually waits for the bed to cool down, which is the way M190 Rnnn should be handled, but not M190 Snnn.

Here is the use case:
In my startup gcodes I first heat up the bed with M190 S40. Once the bed reaches 40 degrees C, I begin to heat up the hotend and further increase bed temperature. This allows hotend and bed to reach the target temperatures almost simultaneously. The way Klipper handles M190 Snnn does not allow to start new print right after completing the previous, or after cancelling the previous job, while the bed temperature is still close to 80 degrees. Printer waits until bed cools down to 40 C (which may take 20-30 minutes) and after that starts heating bed up again. This is waste of time and energy.

So, my question is - would it be possible to change the way Klipper handles M190 Snnn command to make it work like in Marlin? I had no time to test, but if M109 Snnn behaves like M190, would be nice to fix it too.

Thanks
Best regards,
Boris

@KevinOConnor
Copy link
Collaborator

Klipper's goal with g-code support is to be compatible with common 3rd party programs that generate g-code like sli3r, cura, etc. It's common for these programs to have default g-code init sequences that use M109/M190 - they use it to mean "wait for the heater to stabilize at the provided temperature". So, I don't think it makes sense to alter the Klipper g-code M109/M190 support.

That said, if someone wishes to do the development, then I think a new "extras" module that adds a WAIT_ON_TEMPERATURE command would be a fine addition. Such a command could have sufficient additional parameters to fully control when/how the heater heats.

-Kevin

@kantlivelong
Copy link

kantlivelong commented May 25, 2020

Hey @KevinOConnor

I have an open issue opened for OctoPrint-SmartPreheat where by default the tool temps are set in parallel and the bed temp is set in stages.

Example:

M190 S80
M140 S100
M104 S230 T0
M104 S230 T1
M190 S100
M109 S230 T0
M109 S230 T1

What seems to be happening is if printing again immediately after and the bed is already past the first target Klipper will wait for it to cool. From what I understand this shouldn't happen with S mode as it is supposed to be a min temp. It sounds like Klipper is doing is implementing R mode for S mode.

https://www.reprap.org/wiki/G-code#M190:_Wait_for_bed_temperature_to_reach_target_temp

Obviously there is no real gcode standardization but if anything M104|M109|M140|M190 seem fairly standardized.

@FeepingCreature
Copy link
Contributor

FeepingCreature commented Oct 8, 2020

For the Marlin docs as well, M190 S is documented to mean "Wait only for heating". https://marlinfw.org/docs/gcode/M190.html Unless Marlin's implementation diverges from its spec, it seems Klipper's impl rn is actually incompatible with gcode as these firmwares understand it.

edit: Marlin FW interprets S as "wait only for heating": https://github.com/MarlinFirmware/Marlin/blob/2.0.x/Marlin/src/gcode/temp/M140_M190.cpp#L122

edit: Unless the claim here is that Cura/Slic3r actually mean "what Klipper does" rather than "what the biggest firmware on the market does", which seems quite unlikely, especially given that that firmware also supports a separate code for the thing that Klipper does.

@FeepingCreature
Copy link
Contributor

A discussion with the dev on this topic was unfruitful, but made abundantly clear that Klipper is not going to implement this behavior for any reason.

As such, here's a patch:

diff --git a/klippy/extras/heater_bed.py b/klippy/extras/heater_bed.py
index c7f69e50..73c2e2d1 100644
--- a/klippy/extras/heater_bed.py
+++ b/klippy/extras/heater_bed.py
@@ -24,7 +24,14 @@ class PrinterHeaterBed:
             pheaters.wait_for_temperature(self.heater)
     def cmd_M190(self, gcmd):
         # Set Bed Temperature and Wait
-        self.cmd_M140(gcmd, wait=True)
+        # if R, wait for target; if S, wait if below
+        temp = gcmd.get_float('S', gcmd.get_float('R', 0.))
+        pass_if_above = gcmd.get_float('S', None) is not None
+        self.heater.set_temp(temp)
+        if pass_if_above and self.heater.get_temp(0)[0] > temp:
+            return
+        pheaters = self.printer.lookup_object('heaters')
+        pheaters.wait_for_temperature(self.heater)
 
 def load_config(config):
     return PrinterHeaterBed(config)

@denunzio11
Copy link

I'd like to add my support for this request. As someone who prints commercially, time is a factor. The extra several minutes I have experienced waiting for my printers to start their run is punishing (in the wallet). I know this sounds like an exaggeration but I print objects that have several pieces and some have shorter printing times. Adding minutes to each print adds up...I can only imagine what it would do to a farm environment. Just my 2 cents.

@KevinOConnor
Copy link
Collaborator

This seems to come up a bit, so I will expand on my previous comments at #1108 (comment) .

It's extremely common for slicers to issue a M109 S<temp> and M190 S<temp> in their default start scripts. The slicers issue these commands followed by commands that extrude the first layer of the print. This has been the default behaviour for common slicers for over a decade. Extruding the initial layer with an unstable bed or extruder temperature would be a terrible default. Therefore, I feel Klipper needs to continue to treat M109 S<temp> and M190 S<temp> as a wait for stable temperature.

Extruding the first layer while the temperatures are not stable would be a terrible default, because it is common for changes in bed temperatures to impact Z distance and it is common for changes in extruder temperature to impact flow rates and pressure. (For example, many tests have shown that even a couple of degree swings in bed temperature can cause noticeable "z banding" on common printers.) If the first layer is not done with consistent flow rates, Z heights, or temperatures it can impact first layer adhesion, which can result in subtle issues throughout the print (eg, warping, part detachment).

I completely understand that some users would like to customize their heating sequences. As previously indicated, I'm fine with someone contributing code that implements a SET_HEATER_TEMPERATURE WAIT_FOR_MINIMUM_TEMPERATURE=123.4 type command. Users can, of course, also use a gcode_macro to override M109 in their local configs should they choose to. As before, though, I believe the default behavior of M109 and M190 in Klipper should remain as it is.

-Kevin

@FeepingCreature
Copy link
Contributor

FeepingCreature commented Oct 22, 2020

And the notion that people issue these sequences because they don't want the behavior you espouse doesn't enter your consideration, despite the fact that the behavior you describe has a well-established different GCode that is not being used?

This just seems like a one-sided fight between Klipper and slicer devs about how heating ought to work, carried out on the back of GCode interpretation. But it shouldn't be the role of the driver to tell users how they should want heating to work, when those users vocally- and syntactically- disagree.

If you want users to be using WAIT_FOR_STABLE_TEMPERATURE, ie. R, but users are using WAIT_FOR_MINIMUM_TEMPERATURE, ie. S, then this doesn't mean that the "true meaning" of S is actually that of R just because you think that users should use WAIT_FOR_STABLE_TEMPERATURE. If users agreed, they'd be using R already.

Eh. I'm happy running a patched copy. Still a great firmware.

@richfelker
Copy link

It's extremely common for slicers to issue a M109 S and M190 S in their default start scripts

Because users want the behavior which the S form entails. If they want the behavior the R form entails, they'd switch it to the R form.

Extruding the initial layer with an unstable bed or extruder temperature would be a terrible default.

This is what all other firmware does with the S form of the commands, and the way you've framed it is a strawman because it doesn''t mean extruding with unstable temperature. The effect of moving filament out of the nozzle and new cold filament into the nozzle is a far greater destabilization to the control loop than being off by 2 tenths of a degree before there's any filament moving. There is asbolutely no value in trying to reach that kind of stability before doing any extrusion because you lose it immediately. All it does is waste time and electricity.

For the bed, I can understand why one might want to wait longer for the bed temperature to stabilize, but the PID loop is not really telling you that since you don't have sensors all over the bed to measure uniformity. It's just a poor proxy by adding a long delay, and if users want that, they can achieve the same thing by adding a fixed-time delay, or using the R form. There's no reason to break the behavior of the S form.

@KevinOConnor
Copy link
Collaborator

FYI, latest Klipper code supports TEMPERATURE_WAIT sensor=heater_bed minimum=60 style commands (commit e83801d).

-Kevin

@richfelker
Copy link

So can we just define M109 and M190 as gcode macros using that now?

@KevinOConnor
Copy link
Collaborator

So can we just define M109 and M190 as gcode macros using that now?

Yes, it is possible to define a gcode_macro that overrides M109/M190 and calls TEMPERATURE_WAIT.

-Kevin

@bromor
Copy link

bromor commented Dec 22, 2020

I tried TEMPERATURE_WAIT command but faced an error due to missing get_temp attribute.
Console output:
22.12.2020, 14:19:14 !! Internal error on command:"TEMPERATURE_WAIT"
22.12.2020, 14:19:14 // Klipper state: Shutdown
22.12.2020, 14:19:14 {'message': "PrinterHeaterBed instance has no attribute 'get_temp'", 'error': 'WebRequestError'}
22.12.2020, 14:19:14 TEMPERATURE_WAIT sensor=heater_bed minimum=60
22.12.2020, 14:18:20 // Klipper state: Ready

Any hints?
klippy.log

Edit: I just realized that issue #3665 was opened. Can be discussed there.

@KevinOConnor
Copy link
Collaborator

That error should be fixed now (commit a637c2f).

-Kevin

@Block137
Copy link

Try this macro. It has adjustable heat up tolerance.

[gcode_macro M190]
rename_existing: M191
default_parameter_S: off
default_parameter_R: off
variable_tolerance: 1.0
gcode:
    {% if S != 'off' %}
        M140 S{S}
        TEMPERATURE_WAIT SENSOR=bed MINIMUM={S|float - tolerance}
    {% elif R != 'off' %}
        M191 S{R}
    {% else %}
        M140 S0
    {% endif %}

@blerrgh
Copy link

blerrgh commented Apr 6, 2021

I appreciate the TEMPERATURE_WAIT fix, and the M190 macro workaround.

I agree it's most ideal if the PID loop is stabilized before printing starts. However! I think it's fairly bad that, for example, if your bed is stabilized at 100, an M190 S70 command would wait for it to cool all the way to 70.

What if there was some logic to determine the difference between a PID temperature fluctuation and a legitimate heating request? For example, maybe Snn checks if the newly requested temperature is less than, say, 2 degrees below the current temperature, and if so, it proceeds immediately, and if not, it waits for the PID to stabilize.

Or actually, I think it might even work to have Snn always proceed if the requested temperature is less than the current temperature and only wait for a stable PID loop if the requested temperature is greater than the current temperature?

And then implement Rnn with the exact current behavior of Snn, so if people want to cool off a heater they can use Rnn.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants