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

TOGGLE_CHAMBER_FAN is poorly implemented #5

Open
stew675 opened this issue Oct 23, 2024 · 2 comments
Open

TOGGLE_CHAMBER_FAN is poorly implemented #5

stew675 opened this issue Oct 23, 2024 · 2 comments

Comments

@stew675
Copy link

stew675 commented Oct 23, 2024

See here:

gcode.register_command("TOGGLE_CHAMBER_FAN", self.cmd_toggle_chamber_fan)
def cmd_toggle_chamber_fan(self, gcmd):
self.fan_on = not self.fan_on

This creates the TOGGLE_CHAMBER_FAN custom g-code command, but there is no indication of whether the fan was on or off.

Within the get_zoffet macro definition (see here: https://github.com/QIDITECH/QIDI_PLUS4/blob/main/config/gcode_macro.cfg#L29-L45) we can see that TOGGLE_CHAMBER_FAN is called. get_zoffset is called by the G29 macro and others.

The problem is that this is subject to a functional race. If the heater was active prior to G29 being called, then the call to M141 S0 will start to make the chamber fan shutdown. Right now, the chamber fan will stay active for 60s after the chamber heater is turned off. We can assume that this is done to give time for the heater coils to cool down sufficiently.

Now, if the chamber heater was still on when the nozzle wipe sequence completes, but had NOT already reached the target temperature just prior to G29 being called, then the chamber heater coils can still be hot enough to trip the chamber protection sensor in certain scenarios. Some customers have seen this, as the chamber fan can be forced into the off state too soon.

It would be prudent to inject a G4 P15000 command after the M141 S0 command to ensure that the chamber heater unit has had sufficient time to cool down before the fan is forcibly turned off. I recommend that this be done like so:

[gcode_macro G29]
variable_k:1
gcode:
    {% set temp = printer["heater_generic chamber"].target %}
    M141 S0
    {% if temp > 0 %}
        G4 P15000
    {% endif %}
    BED_MESH_CLEAR

Additionally, the behavior of TOGGLE_CHAMBER_FAN is inherently relying on the internal coding of chamber_fan.py to ensure that the fan, if initially off, won't turn on because the heater was previously disabled with M141 S0. It's just bad programming practice as it leaves the fan in a non-determinant state. Who knows who/what may have called TOGGLE_CHAMBER_FAN in the past?

I modified chamber_fan.py to give an explicit enable/disable gcode command like so:

        gcode.register_command("TOGGLE_CHAMBER_FAN", self.cmd_toggle_chamber_fan)
        gcode.register_command("ENABLE_CHAMBER_FAN", self.cmd_enable_chamber_fan)
        gcode.register_command("DISABLE_CHAMBER_FAN", self.cmd_disable_chamber_fan)
    def cmd_toggle_chamber_fan(self, gcmd):
        self.fan_on = not self.fan_on
    def cmd_enable_chamber_fan(self, gcmd):
        self.fan_on = True
    def cmd_disable_chamber_fan(self, gcmd):
        self.fan_on = False

It seems to me that get_zoffset wants to be calling DISABLE_CHAMBER_FAN at the start of the macro, and calling ENABLE_CHAMBER_FAN at the end of it, and that's the way it really should be, not potentially flipping the fan state in a non-deterministic fashion and trusting other code paths to handle such properly.

@QIDITECH
Copy link
Owner

Thank you for your advice.

@nicwilson58
Copy link

thanks Stew675, I agree and I have made these changes to mine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants