I think this is broadly a good idea. That said, I think you can do this manually during firmware build time by configuring the fan pin to be set high.

No, you can't. That's why I went to the trouble to figure out how to do this. Klippy host side explicitly turning the fans off and disconnecting cleanly or whatever prevents the mcu side from turning them on in this situation.

No, you can't. That's why I went to the trouble to figure out how to do this. Klippy host side explicitly turning the fans off and disconnecting cleanly or whatever prevents the mcu side from turning them on in this situation.

Oh, right - this is a restart and not a firmware restart. Just to confirm, you don’t also see this behavior on firmware restarts when the pin is set high during build time, correct?

I'll have to run a test with this reverted to check what happens on firmware restart. Initially I thought of just making restart a macro for firmware restart, but actually getting to the bottom of the problem seemed cleaner. Normally I only use firmware restart if normal fails because mcu is in a locked shutdown state.

Thanks. I understand why this could be useful, but this looks like a notable change in behavior. (Notable in that users will definitely notice it.) It's not clear to me that it is an overall improvement - it is not obvious to me that a "shutdown speed" would be applied at restart.

Cheers,
-Kevin

The reason to apply it at restart is that restart is a state where klippy is not in control of the MCU. The expectation is that it will quickly take control again, but that doesn't always work. I've had damage to printers from hotend fan not running when it should be, and while I don't recall if failure-to-restart or another cause was responsible, it's a risk I'm always concerned about, because if "restart" does somehow fail and you lose the control channel to the MCU, it's not clear how to get the fans running again in time to avoid a huge amount of heat getting where it doesn't belong.

The original change I prototyped was just removing the fan-off-at-restart entirely, but I thought honoring shutdown speed would be less invasive. If it's deemed invasive, another option I think would be very reasonable is this:

At restart, only reduce the fan speed down to shutdown speed. If it's already off or lower, leave it alone. This would make it so that fans with a shutdown speed of 0 all behave exactly as they do now, but so the hotend fan stays in the state if was on when the reset was commanded.

Does that sound reasonable? If so I can submit an updated version.

Thanks. I do understand why this could be useful. Alas, I don't know what the best thing to do is. My main points are:

• It would be a noticeable change in behavior.
• Mixing of "shutdown" and "restart" does not seem like an improvement to me. A "shutdown state" is an error state and it seems odd to mix that with a non-error state like the "restart state".
• I don't think the current behaviour is incorrect and changing it is not a priority for me. To wit, if a user unplugged/plugged the printer they should have no expectation that the printer continues to immediately do what they want it to do, and similarly if a user issues a RESTART I don't see how they can have any similar expectations. Fan speeds are just one of many things that could go wrong if one issues a RESTART while the machine is active.

Cheers,
-Kevin

Just to be clear, I'm not against making a change here. I don't know what change would make sense though. I fear just turning on some fans (or keeping them on) could have annoying side effects that make some users unhappy.

Cheers,
-Kevin

• To wit, if a user unplugged/plugged the printer they should have no expectation that the printer continues to immediately do what they want it to do

If I unplug and replug the printer, it will safely bring up my fan pins the way I compiled the firmware to do. But if I type restart, it won't. That's the least-surprise-violating behavior here.

In fact, I'm rather confused why the fan module has the on-restart handler to begin with. Even if we got rid of it entirely, the only change users would notice in the non-fault case is that it takes a couple seconds longer for the part cooling fan to turn off (until klippy control of mcu is re-established), and that if the hotend fan was on, it doesn't cycle off/on for a few seconds.

Would you be amenable to just removing the handler? I'm trying to think of a good reason to have it and can't think of any. At first I thought it was because the fan might be software pwm at a lower-than-100% level and that the restart could leave the pin fully on, but the MCU isn't shutdown so that shouldn't happen. If the MCU somehow did shutdown, it would put the fan into whatever the shutdown state for it is.

If I unplug and replug the printer, it will safely bring up my fan pins the way I compiled the firmware to do. But if I type restart, it won't. That's the least-surprise-violating behavior here.

That's fair. The initial pins are exported by the mcu (mcu.get_constants().get("INITIAL_PINS")) and it seems reasonable to alter behavior based on that.

In fact, I'm rather confused why the fan module has the on-restart handler to begin with.

Looking through the commit history it looks like it dates back to 29131c8 - basically the start of Klipper. It looks like it was initially targeted at the layer cooling fan, as this pre-dates support for other fans. As more fans were added, they reused the same base code.

Would you be amenable to just removing the handler?

With behavior changes like that, I fear it makes someone else unhappy. That said, I agree that it seems odd that heater_fan goes out of its way to turn itself off. Maybe just exempt heater_fan from the current behavior?

-Kevin

+1 to exempting some fans from turning off during restart. E.g. [heater_fan], [temperature_fan], perhaps [controller_fan]. Motivation being that the fans that are temperature-controlled, if enabled, were enabled for a reason, and it is unreasonable to expect that during restart the temperature condition would magically go away. And since Klipper has no way of checking the temperature conditions during restart, it is best to leave the fans on, since after the restart Klipper will be able to re-evaluate the temperature conditions and either keep the fans on or turn them off. [controller_fan] and [fan_generic] are a bit more difficult to argue here, since they would typically not be reactivated upon restart, so it may be OK to turn them off during restart too.

I'm okay with that.

Cheers,
-Kevin

I'm okay with that too, but "has a nonzero shutdown speed" is an excellent proxy for "is a heater or controller fan that probably needs to stay on when we can't control it". Otherwise you have to do some elaborate hard-coding of specific types of fans that should suppress the on-restart logic to turn them off.

As mentioned above, if having them suddenly start turning on during restart would be annoying to users, we can avoid turning them on, but just skip turning them off if the shutdown speed is nonzero (leave them in whatever state they were in prior to the restart command in this case).

but just skip turning them off if the shutdown speed is nonzero (leave them in whatever state they were in prior to the restart command in this case)

I think this logic should be fine too.

I don't think it is a good idea to tie shutdown speeds to restarts. It seems confusing to me.

As for the implementation, one could add a disable_on_restart=False parameter to the Fan() class (right next to default_shutdown_speed) and PrinterFan, GenericFan, etc. can turn it on.

-Kevin

As for the implementation, one could add a disable_on_restart=False parameter

That sounds like a perfectly workable solution. Would it be exposed in the config file or just hard-coded for each type of fan? Since generic fan together with delayed gcode probing conditions can be used to implement various kinds of temperature-controlling fan setups, I think it would be preferable to have some way users can ensure a particular generic fan doesn't get shut off at restart. But if this is too much to ask, just getting it right for the hotend fans would be a good start at least...

I'd say the setting doesn't have enough value to justify a new config entry. I'd just put it in the code.

-Kevin

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.