Asynchronous temperature setting, and cancel of temperature wait

[KevinOConnor]

This PR adds a new heaters/set_target_temperature API endpoint. It allows the frontends to asynchronously set a new target temperature. Currently, temperatures are set via M109, M104, M190, M140, and SET_HEATER_TEMPERATURE commands. However, these commands wait in the G-Code command queue, which can be delayed - in particular if the printer is waiting for another temperature to be met. In contrast, the new API call sets the requested target temperature as soon as it is received.

To avoid conflicts, if a heater target temperature is set asynchronously through the API while a command is actively waiting for that heater to reach a temperature (TEMPERATURE_WAIT, M109, M190) then the new asynchronous temperature will be accepted and the waiting command will raise an error. So, for example, if the user ran M190 S200 and then issued a {"method": "heaters/set_target_temperature", "params": {"heater":"heater_bed", "target": 60.0}} then the target temperature will be set to 60 and the M190 command will report an error. (If the command was issued from a virtual_sdcard based print then that error would cause the on_gcode_error code to run which may change the target temperature again.)

To alert API users of this conflict, this PR also introduces new printer.heaters.temperature_wait status information. If Klipper is actively waiting for a temperature (TEMPERATURE_WAIT, M109, M190) it will report that heater (or sensor) involved in the wait. It will report null in the common case where no temperature wait is in progress.

These two mechanisms can also be utilized to "cancel a heater wait". An API session could determine if the code is waiting on a temperature and then purposely issue an asynchronous temperature assignment to cause the waiting command to exit early (with an error).

FYI, @meteyou , @pedrolamas , @Arksine .

Thoughts?
-Kevin

[meteyou]

hey @KevinOConnor! Thx for the ping and this feature!

I started with the implementation with the display of "temperature_wait" and I took a closer look at the behavior.

i would like to continue to set normal temperature changes with SET_HEATER_TEMPERATURE, because this command is also in the console and only an "overwriting" of a target temp while Klipper is waiting for it with the api call. do you think this implementation is also correct way?

[KevinOConnor]

i would like to continue to set normal temperature changes with SET_HEATER_TEMPERATURE, because this command is also in the console and only an "overwriting" of a target temp while Klipper is waiting for it with the api call. do you think this implementation is also correct way?

Yes - the SET_HEATER_TEMPERATURE command is still available and still a useful way to set the temperature. The proposed heaters/set_target_temperature API provides some additional flexibility, but it is not required.

-Kevin

[meteyou]

We have now thought internally (Mainsail-Crew) about using this new API call and have found two use cases.

use case 1: cancel print during the preheat (START_PRINT)
You could interrupt the temperature wait and set the target temperature to 0 for each temperature_wait object. Then, the API call will trigger an on_gcode_error that triggers a CANCEL_PRINT.
Problem: we must trust that the on_gcode_error is set correctly or send another command afterward.

use case 2: adjust bed or extruder temp during a preheat
if you have selected an incorrect bed temperature when slicing, that you overwrite it when heating up.
Problem: executing on_gcode_error will execute a TURN_OFF_HEATERS in the Klipper default, and if it is a KIAUH or MainsailOS installation, it will execute a CANCEL_PRINT. So, the print will stop immediately after the adjustment.

Do you have a better suggestion for implementing these two use cases, or do I misunderstand the API call?

[KevinOConnor]

The main use case I had in mind was changing the temperature without waiting for other actions to complete. For example, if one sets the extruder to 240, sets the bed to 80, and then waits on the bed temperature - then this PR would allow one to change the extruder temperature (eg, set 210) without having to wait for the bed to fully heat first.

Problem: we must trust that the on_gcode_error is set correctly or send another command afterward.

What follow up commands are required? If the print has issued a "wait for heater_bed to reach 60" and mainsail sends an "asynchronous set heater_bed to 0 command" then the target temperature for heater_bed will immediately be set to zero (ie, off). Then the "wait for heater_bed command" will raise an error, on_gcode_error will run, it will (likely) set the heater_bed temperature to zero (which is redundant but harmless). So, the heater should be off and the sdcard print will no longer progress.

Problem: executing on_gcode_error will execute a TURN_OFF_HEATERS in the Klipper default, and if it is a KIAUH or MainsailOS installation, it will execute a CANCEL_PRINT. So, the print will stop immediately after the adjustment.

I guess that is what I'd expect to happen in that case. What action is preferred?

I guess, if I issue a command to "set the bed temperature to 80 and wait for that temperature" (ie, M190 S80) then it would seem surprising to me that the command would silently complete without actually waiting for a temperature of 80. In general, I'd want a command to do what it is requested to do, or report an error because it can not perform the requested action.

To be clear, this PR is open for discussion. We can certainly change the details, implement an alternative, or skip it.

Cheers,
-Kevin

EDIT: Other use cases would be changing temperatures during bed_mesh, quad_gantry_level, and other long running startup commands.

[meteyou]

In use case 1, we would prefer the command CANCEL_PRINT (move the toolhead to the parking position, disable all heaters, disable steppers...). This should be the case for every KIAUH and MainsailOS instances.

In use case 2, every on_gcode_error call will stop the print (TURN_OFF_HEATERS will break the print and CANCEL_PRINT would also break the print), and the possibility of changing the temp would be "useless".

Calling on_gcode_error is the main problem in every use case, as it executes "unexpected" or "unwanted" commands. So we would prefer just a warning in the console when the API call interrupts M109/M190/TEMPERATURE_WAIT instead of the on_gcode_error.

EDIT: Other use cases would be changing temperatures during bed_mesh, quad_gantry_level, and other long running startup commands.

Is there a good possibility of detecting that such a command like quad_gantry_level is being executed? I don't think there is an entry in printer.heaters.temperature_wait or something similar that we can read out to decide whether a normal gcode or this API call should be sent.

[pedrolamas]

I do see value in what is proposed here, however I do agree with @meteyou that the fact on_gcode_error will be run if this interrupts a "wait for temp" operation might be a problem as that is basically killing any running print (given what we set as default for this)

[KevinOConnor]

Okay - thanks for the feedback.

I'm not entirely sure I understand the alternate implementation being suggested. It sounds like what is being proposed is that an asynchronous temperature change would not cause an existing "wait on temperature" (eg, M190) to raise an error. Instead the existing wait command would complete early and any print in progress would proceed anyway (perhaps after writing something to the log or g-code console).

FWIW, exiting the wait command early without raising an error does not seem viable to me, as it would have nasty failure situations. Consider the case where a user issued an M109 S260 and realized they actually wanted to print at 240. If the user asynchronously altered the temperature to 240 and the wait command completed early, then the print might start before the extruder was at either temperature - possibly clogging the nozzle. Unless I'm missing something, the code has no way to know what the "user really wants" (eg, stop heating and stop printing, continue heating but proceed anyway, stop heating but proceed anyway, continue heating but stop printing).

It does sound like there isn't consensus on this change though. There might still be value in supporting asynchronous temperature changes, but it seems that it is probably best to not mix it with "cancelling" of an existing wait on temperature.

Is there a good possibility of detecting that such a command like quad_gantry_level is being executed? I don't think there is an entry in printer.heaters.temperature_wait or something similar that we can read out to decide whether a normal gcode or this API call should be sent.

Just "thinking out lout", we could add a mechanism for Klipper to announce it is in a long running operation. Maybe something like printer.gcode.long_running_commnd - that may be a better idea than adding a new printer.heaters.temperature_wait. Not sure.

Thanks again,
-Kevin

[whyme12]

FWIW, exiting the wait command early without raising an error does not seem viable to me, as it would have nasty failure situations. Consider the case where a user issued an M109 S260 and realized they actually wanted to print at 240. If the user asynchronously altered the temperature to 240 and the wait command completed early, then the print might start before the extruder was at either temperature - possibly clogging the nozzle. Unless I'm missing something, the code has no way to know what the "user really wants" (eg, stop heating and stop printing, continue heating but proceed anyway, stop heating but proceed anyway, continue heating but stop printing).

Just for clarification. If something like this is added to on_gcode_error:

on_gcode_error:
{% if printer.heaters.temperature_wait != null %}
  {% if printer.heaters.temperature_wait == M109 %} M109 S{printer.extruder.target} {% endif %}
  {% if printer.heaters.temperature_wait == M190 %} M190 S{printer.heater_bed.target} {% endif %}
  # TEMPERATURE_WAIT will be tricky due to min and max
{% else %}
  CANCEL_PRINT
{% endif %}

I know, code above needs more stuff, like cancel print when target temperature was set to 0.

Lets say the printer is in a start macro, heating up the bed, the user notices it was a old gcode file and the new material needs 5°C more bed tempererature.
While the bed is heating, user issues a asynchronoues set bed temperature to 5°C more.

Will the above even work? If yes, what will happen? The wait in start macro is canceled, on_gcode_error is called -> the above code will wait for new target.
And then? Will the start macro continue at the point it was interrupted or will the whole macro be canceled?

If the macro would continue, then a different exit path would be better. Instead of on_gcode_error, maybe something like on_wait_interrupt with parameters of the command that was interrupted and the new temperature target, if thats possible.

If the macro would be canceled anyways. Print a warning and ignore the change of the temperature would be enough and additionaly adding a async cancel print would be better in my oppionion (no need to emergency stop and not losing home etc.)

[meteyou]

It does sound like there isn't consensus on this change though. There might still be value in supporting asynchronous temperature changes, but it seems that it is probably best to not mix it with "cancelling" of an existing wait on temperature.

I have misunderstood the API call until now and therefore misinterpreted the on_gcode_error. I thought that the command only changed the target temperature. So if the user starts a print with extruder 240, but realizes during heating that he wanted 260, the API call overwrites the target of M109, and Klipper just heats up to 260 and then continues the rest of the code!
In my opinion, the API call should only cancel the wait if the new target temperature is 0.

Would such an implementation be possible?

Just "thinking out lout", we could add a mechanism for Klipper to announce it is in a long running operation. Maybe something like printer.gcode.long_running_commnd - that may be a better idea than adding a new printer.heaters.temperature_wait. Not sure.

I love the implementation of printer.heaters.temperature_wait because you can show the user a "wait for temperature". Whether this API call will be implemented or not, the visualization for such a wait alone would be an advantage for the user.
Some users have already asked in the support channel why their print does not start, and we could attribute this to bad PID values and Klipper was inability to "stably reach" the target temperature. With the output of printer.heaters.temperature_wait, this would also be visible for the user in the GUI.

[KevinOConnor]

Hi,

I have misunderstood the API call until now

Just to be clear, the code on this PR does three main things:

1. Allow one to set a heater target temperature asynchronously via the api server. That is, it allows one to set the target temperature without having to wait for the current g-code queue to complete.
2. It changes the M109, M190, and TEMPERATURE_WAIT commands to check if the heater that it is waiting on is commanded asynchronously - if that occurs the waiting command handler will raise a g-code error. This occurs if any asynchronous temperature is set for that heater (eg, set to zero, set to a higher temp, set to the same temp).
3. printer.heaters.temperature_wait is added.

The reason I added the error check to M109, M190, and TEMPERATURE_WAIT is that I felt the alternative would have a notable failure situation. Consider the case where a user issues M109 S260 and then asynchronously sets the temperature to 240. Without a check in the waiting code the heater would likely settle on 240 and wait forever never reaching 260. Neither the idle timeout checks nor the verify heater checks are likely to trigger, and there is a good chance the heater could potentially stay in that state for hours, days, weeks, etc. I fear that would be surprising behaviour for a user.

In my opinion, the API call should only cancel the wait if the new target temperature is 0.

That doesn't sound like it would work well in the failure situation above.

Changing the target of the wait is also problematic. It is probably technically possible for M109 and M190, but it would be difficult to implement for TEMPERATURE_WAIT as that command is often set to wait for a temperature other than the target. So, no way to know what a new wait temperature should be. Changing the wait for temperature also seems like it could be surprising for a user - for example, if the user requests M109 S240 and then asynchronously changes the target to 260, it is not clear that the user wants to wait for 260. In general I'm leery of trying to write code to infer "what the user really wants" as, at least for me, "I really want the printer to do what I tell it to, or tell me it can't do that". So, for example, if the user really wants to wait for temperature 260 then it seems like that would best come in some kind of command that states that explicitly.

Happy to change this PR though or go a different direction.

I love the implementation of printer.heaters.temperature_wait because you can show the user a "wait for temperature".

Okay, thanks. I'm fine with merging that into mainline Klipper.

Cheers,
-Kevin

[meteyou]

Changing the target of the wait is also problematic. It is probably technically possible for M109 and M190, but it would be difficult to implement for TEMPERATURE_WAIT as that command is often set to wait for a temperature other than the target. So, no way to know what a new wait temperature should be. Changing the wait for temperature also seems like it could be surprising for a user - for example, if the user requests M109 S240 and then asynchronously changes the target to 260, it is not clear that the user wants to wait for 260. In general I'm leery of trying to write code to infer "what the user really wants" as, at least for me, "I really want the printer to do what I tell it to, or tell me it can't do that". So, for example, if the user really wants to wait for temperature 260 then it seems like that would best come in some kind of command that states that explicitly.

This is also why I only want to send this API command if a "set temp & wait" gcode is already active for this particular heater. For any normal state (just set a target without a wait), I would just set the target temperature via the SET_HEATER_TEMPERATURE (the standard behavior when you set a target in the temperature panel).

We could also add a prompt in the GUI to ask the user if they want to "set & wait" or "queue the command in the gcode queue". But this confuses the user more than it helps them.

However, this decision should not be made or blocked by the API call (in Klipper); in my opinion, the API call should "simply work". Too much logic in this API call could only lead to too many side effects, making it no longer helpful for the user.

It is probably technically possible for M109 and M190, but it would be difficult to implement for TEMPERATURE_WAIT as that command is often set to wait for a temperature other than the target.

I don't know if I can differentiate this properly in the GUI (target heater temp or temperature_wait). I have to test this use case to give a good answer. Right now, I'm talking only about the M109 and M190 gcodes. Maybe I have time this weekend to give here a better feedback.

[Sineos]

As a simple user (and likely all that have a Marlin background), I would expect to simply overwrite any previous "set and wait" with a new value from the webifs.
I guess the typical expected flow would be:

1. M109 S200 (wherever it may come from: slicer, start_gcode, manual etc)
2. Realize that I wanted 220 °C
3. Set a new temperature (preferably also with the known M109 S220)
4. "Something" in background checks if there is an active "set and wait" running. If YES, then cancel it
5. "Something" sets the new "set and wait" to 220 °C

If there are any more complex API calls involved in the background, I'd expect the webifs to take care of whatever is needed.

[meteyou]

I have implemented this function so that you can see it live. So, when printer.heaters.temperature_wait is not null, the API command is used instead of the Gcode.

You can download a Mainsail build from the CI workflow (https://github.com/mainsail-crew/mainsail/actions/runs/10334206411?pr=1967). Just unzip the ZIP file and exchange the content of the folder /home/pi/mainsail with it. After a force refresh (ctrl+shift+F5), you have the test build and can play around with the API command. (You will also see a fire icon with a "!" behind the heater with a current temperature_wait)

I noticed two things during this test. The first has nothing to do with the command itself, but since there was no printer.heaters.temperature_wait until now and thus was not visualized, I just noticed it:
I started a print and expected to see the icon for the temperature_wait at the extruder and the heater_bed, but it was only at the extruder. At first, I thought it was a bug, but when I looked at the gcode, I realized the reason for this behavior. Here is the gcode (snipped from my start_gcode macro):

M104 T0 S150
M140 S{bed}
M109 T0 S150
M190 S{bed}

It just waits for the extruder, and after that, it starts waiting for the bed. So, it's not a bug, but I think this will also create the same questions in the support channels when this feature is published.
I also wonder if you could do something in Klipper to wait for both temperatures "simultaneously". This would also help with overwriting the temperature in this period because when I change my bed temperature, it will automatically be overwritten after the extruder temperature has reached its target because the gcode command (M190 S100) is simply read. (I'm just thinking out loud... so this is not a bug or something similar)

Now, let's move on to the second problem I noticed. Because we are not sending gcode here, the change does not appear in the console. An output would always be desirable here. However, I would make the output more "accurate" than the current error output.

This is the current error output:

Heater 'heater_bed' target temperature changed during wait

This is how I would like the output to be:

Heater 'heater_bed' target temperature changed from 100°C to 110°C during wait

But if you now play around with the test-build and test the feature in live mode, you will probably recognize our problem with the on_gcode_error. Using the command without every print being aborted is simply impossible.

[tobyfu]

As it is currently I think you would want to use the api call for setting the temperature only when temperature_wait is null (to avoid it erroring and aborting the print unintentionally).

It might be good for it to show a confirmation box when temperature_wait is not null so that a user can choose either to issue it async (which will error and abort the print) or alternatively issue the gcode which will apply after the wait and any other queued commands.

[KevinOConnor]

I have implemented this function so that you can see it live.

Thanks for working on this.

I started a print and expected to see the icon for the temperature_wait at the extruder and the heater_bed, but it was only at the extruder. At first, I thought it was a bug, but when I looked at the gcode, I realized the reason for this behavior.

Good point. This is definitely one of the challenges with anything that involves "cancelling" or "altering" the command stream - the behaviour can be subtly different from the expected behaviour in a way that can cause notable surprise. For example, when I lasted attempted "cancel" logic I ran into the issue described at: #4398 (comment)

I guess we could introduce new commands, but for what it is worth I'm not sure that would be worth the effort.

This is how I would like the output to be:

Okay, that's simple enough to change.

Using the command without every print being aborted is simply impossible.

Okay, thanks for the feedback. I'll give it some further thought.

For what it is worth though, I'm thinking this change may not be suitable for the gui frontends. Mixing asynchronous and synchronous commands is likely to be error prone in general (for example, if a synchronous change is made followed by an asynchronous change, then the two updates might get applied in the inverse order). I also now suspect that attempting any kind of "cancel" via an asynchronous update is also going to be error prone and confusing. In particular, I'm not sure there is consensus on what a "cancel" should do.

Thanks again,
-Kevin

[meteyou]

@tobyfu

As it is currently I think you would want to use the api call for setting the temperature only when temperature_wait is null (to avoid it erroring and aborting the print unintentionally).

I would currently only use the command if temperature_wait is set for another heater, but not for the current one I want to change. Otherwise, the command makes no sense because if no temperature_wait currently exists, I can send the standard gcode command without the risk of the print being canceled.

@KevinOConnor

Good point. This is definitely one of the challenges with anything that involves "cancelling" or "altering" the command stream - the behaviour can be subtly different from the expected behaviour in a way that can cause notable surprise. For example, when I lasted attempted "cancel" logic I ran into the issue described at: #4398 (comment)

In the case of a cancel, I would see the execution of on_gcode_error (mostly a CANCEL_PRINT) as very effective. Perhaps you could also add an on_gcode_cancel, which I would personally set to CANCEL_PRINT again.

This gcode will already used for failed bed leveling commands (QGL, z-tilt, bed_mesh), which works very well. All reasonably current setups use mainsail.cfg/fluidd.cfg (https://github.com/mainsail-crew/mainsail-config/blob/master/client.cfg#L63) which covers this very well.

But if a user also wants a unique solution, he/she can do the same by adding his/her gcode to on_gcode_error(or on_gcode_cancel if you want to separate it). I think that Klipper cannot offer a "generally valid build-in solution", because there are too many different setups, and with the possibility to provide a user-defined gcode here, the user is free to shift from the default and develop his/her own solution.

You don't have to overthink "What should the printer do after canceling. What is wrong and right?" but rather leave this step to the user. We (Mainsail-Crew) only offer the user a "starting point" here, which he/she can change at any time. Just my 2 cents...

For what it is worth though, I'm thinking this change may not be suitable for the gui frontends. Mixing asynchronous and synchronous commands is likely to be error prone in general (for example, if a synchronous change is made followed by an asynchronous change, then the two updates might get applied in the inverse order). I also now suspect that attempting any kind of "cancel" via an asynchronous update is also going to be error prone and confusing. In particular, I'm not sure there is consensus on what a "cancel" should do.

In general, an asynchronous method could have a positive impact, especially for frontends (if implemented correctly). I often see requests pointing in this direction, and we can only respond with, "Mainsail uses only gcodes, which are queued..." (this is most often requested for controlling the part fan). I've also seen some very complex macro collections to "block" these synchronous commands from the G-code file and only use the frontend to change it and more...
Therefore, to improve usability, I prefer more asynchronous commands for the frontends.

[github-actions]

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.