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

Request for security - PID autotune #12679

Closed
HuguesDug opened this issue Dec 19, 2018 · 11 comments
Closed

Request for security - PID autotune #12679

HuguesDug opened this issue Dec 19, 2018 · 11 comments

Comments

@HuguesDug
Copy link

Good morning,

Last week-end I converted my Anet A8 from Marlin 1.1.8 to 1.1.9.

I started the PID autotune calibration by using the M303 command. But, entering the wrong S value (1850 instead of 185).

I then discovered that there is no max temperature inside the firmware that would protect the equipment. The extruder went up to 600°C in no time, before I jumped onto the power plug.

I would recommand to set a max level inside the firmware so, even in case of an error in M303, you don't burn your house !

Thanks.

Hugues

@RudyBenner
Copy link

Yikes.

@rmoravcik
Copy link
Contributor

Marlin has already MAXTEMP value for each heater in Configuration.h, so it could be easy to add check to M303 command to respect those values.

// When temperature exceeds max temp, your heater will be switched off.
// This feature exists to protect your hotend from overheating accidentally, but *NOT* from thermistor short/failure!
// You should use MINTEMP for thermistor short/failure protection.
#define HEATER_0_MAXTEMP 275
#define HEATER_1_MAXTEMP 275
#define HEATER_2_MAXTEMP 275
#define HEATER_3_MAXTEMP 275
#define HEATER_4_MAXTEMP 275
#define HEATER_5_MAXTEMP 275
#define BED_MAXTEMP 120

For example. if you try to run PID tunning from LCD menu, the max. heater temperature is already limited to value MAXTEMP - 15.

@Ludy87
Copy link
Contributor

Ludy87 commented Dec 19, 2018

Wrong, this security function is not stored in the console input

M303 M104 M109 M140 M190

@forkoz
Copy link
Contributor

forkoz commented Dec 20, 2018

Whoa, who made your heaters that they can do 600C. Can the temp sensor even read that? All the cheap stuff barely reaches the low polycarbonate ranges but you could be printing in PEEK. :)

Yea, all these functions should be respecting maxtemp if they are not.

@rmoravcik
Copy link
Contributor

M104/109 and M140/190 should be already protected by HEATER_x_MAXTEMP and BED_MAXTEMP.

It's true, that you can set a higher temperature than the limit set in Configuration.h (using these gcodes), but ThermalManager will refuse to heat up hotend or bed above those limits.

So, the only problem is with M303.

@RudyBenner
Copy link

RudyBenner commented Dec 20, 2018

I guess the thing to do now is to test . No, I do not wish to have a smoldering pile of wreckage, so I am lowering maxtemp for the test.

#define HEATER_0_MAXTEMP 170 //280 //275

Indeed, I can exceed MAXTEMP with M303. Goes into a normal PID tune routine.

@HuguesDug
Copy link
Author

@rmoravcik
Possible to include in bug-fix 1.1.9 ?

Thanks

@rmoravcik
Copy link
Contributor

rmoravcik commented Dec 21, 2018

@HuguesDug Pull request created #12703. I hope it will be merged soon.

@boelle
Copy link
Contributor

boelle commented Feb 21, 2019

@HuguesDug problem solved?

@HuguesDug
Copy link
Author

Hello,

I did not test the fix because I did not feel like trying to burn the machine a second time. But having read the code modification, I am sure it works.

Thanks for this security fix.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2020
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

7 participants