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

Do not disable extruder on exit #1415

Open
PKav89 opened this issue Feb 19, 2024 · 8 comments
Open

Do not disable extruder on exit #1415

PKav89 opened this issue Feb 19, 2024 · 8 comments

Comments

@PKav89
Copy link

PKav89 commented Feb 19, 2024

Hello!

If Printrun gets closed it disables extruder heating even if printer is currently printing something from SD Card. Such behavior can ruin print if not noticed in time. This should be removed.

self.p.send_now("M104 S0.0")

@DivingDuck
Copy link
Collaborator

Well, I'm not sure if we should change the behavior. When a print was started from Pronterface it is controlled by Pronterface. In case the user call Pronterface to exit during a print the program should take care that at least the printer is not in an situation that heaters are heating in an uncontrolled manner. For me this is more a safety feature and intended behavior.

When I not want Pronterface controlling the printer during a print I would start the print from the printer itself and do not print via Pronterface.

For interest, why would you want to exit Pronterface during a print?

@PKav89
Copy link
Author

PKav89 commented Feb 19, 2024

I connected to printer to change max speed, accelerations and some other settings. It's much faster than via menu.

At least, Printrun should ask if user wants to turn off heater or implement option in settings window.
Safety is a firmware's job.

@rockstorm101
Copy link
Collaborator

I agree with @DivingDuck here, I think shutting everything down is a sane default behavior for a host when it disconnects from the printer. So the printer is left on a sort of "neutral" state. In my case, Printrun is the only controller for the printer, if it didn't shut everything down, nobody else would. Changing this behavior feels like a breaking change because users might be already used to Pronterface turning things off and they would expect it to keep doing that on disconnecting.

I connected to printer to change max speed, accelerations and some other settings. It's much faster than via menu.

I don't really see this use-case, did you really connect mid-print to change max speeds and accelerations? Sounds like asking for trouble. Doesn't mixing controlling hosts and expecting no issues sound a bit naive? Note that Pronterface has no way to know whether a print was started by another host (or the printer itself). Nonetheless, you are right to point out that we do not cover multi-hosts properly (or at all). If there was a big user base on this situation I guess we might want to address it somehow.

All that being said, looking at the current code for pronsole.do_exit I think we should make this change to ask for confirmation before turning heaters off:

     def do_exit(self, l):
+        if self.p.printing and l != "force":
+            self.log(_("Are you sure you want to exit while printing?\n\
+(this will terminate the print)."))
+            if not self.confirm():
+                return
         if self.status.extruder_temp_target != 0:
             self.log("Setting extruder temp to 0")
         self.p.send_now("M104 S0.0")
         if self.status.bed_enabled:
             if self.status.bed_temp_target != 0:
                 self.log("Setting bed temp to 0")
             self.p.send_now("M140 S0.0")
         self.log("Disconnecting from printer...")
-        if self.p.printing and l != "force":
-            self.log(_("Are you sure you want to exit while printing?\n\
-(this will terminate the print)."))
-            if not self.confirm():
-                return
         self.log(_("Exiting program. Goodbye!"))
         self.p.disconnect()
         self.kill()
         sys.exit()

@PKav89
Copy link
Author

PKav89 commented Feb 20, 2024

Well, yes, there is no reason for completely removing this feature. Prompt on exit like "Heater is on. Do you want to disable all heaters before exit?" would be nice too.

@DivingDuck
Copy link
Collaborator

All that being said, looking at the current code for pronsole.do_exit I think we should make this change to ask for confirmation before turning heaters off:

Yea, I saw this and thought that as well. I was even more surprised at when looking to the code. I did yesterday night a quick test and was curious why only the extruder temp was set to zero. Surprisingly the bed temp was still on and not something I had expected. Maybe you can check this on an different printer. My installation is a bit different with more than one print bed and still uses Smoothieware which do not become much love from the developers these days. I'm not sure if this is a general problem or happens only on my specific system when printing via SD.

Regarding the additional "are you sure" behavior, as this is a part of the command line tool Pronsole we usually do not want to have such user actions in addition. I also understand the point from @PKav89 and think we can maybe extend the existing prompt with a "All heaters will set to 0" or something similar so that a user know about the implying consequences.

@PKav89
Copy link
Author

PKav89 commented Feb 20, 2024

What about option "Disable all heaters upon exit", which is on by default?

@DivingDuck
Copy link
Collaborator

Hi @rockstorm101,
did you already change the code? If not, I can do that in the translation pr because of the change for the log string translation.

I like to change the log string to @PKav89 suggestion "Are you sure you want to exit while printing?\n\Disables all heaters upon exit." if you are fine with this.

@rockstorm101
Copy link
Collaborator

Hi @rockstorm101, did you already change the code? If not, I can do that in the translation pr because of the change for the log string translation.

I like to change the log string to @PKav89 suggestion "Are you sure you want to exit while printing?\n\Disables all heaters upon exit." if you are fine with this.

I did not make any changes so please go ahead. I'm happy with your suggestion.

What about option "Disable all heaters upon exit", which is on by default?

I would not close this issue with the change above though. I want to have a think about implementing this option.

DivingDuck added a commit to DivingDuck/Printrun that referenced this issue Mar 7, 2024
…re, make the consequences of exit a print more transparent kliment#1415
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants